Skip to content

Fix rec index in plot_mag and plot_zed #847

Merged
apivarunas merged 1 commit into
masterfrom
pmagplotlib_rec_index_fix
May 6, 2026
Merged

Fix rec index in plot_mag and plot_zed #847
apivarunas merged 1 commit into
masterfrom
pmagplotlib_rec_index_fix

Conversation

@Swanson-Hysell

Copy link
Copy Markdown
Member

Summary

Commit 81a4339 (2026-04-16) included a useful plot_zij pandas/matplotlib compatibility fix. However it seems like it also included changes that are problematic for plot_mag and plot_zed.

  • In plot_mag and plot_zed, revert rec[4] back to rec[5] for the quality-flag check. Every datablock built in the codebase puts the quality flag at index 5 (index 4 is the type/blank column), so with rec[4] the check was never true — plot_mag was dropping every point from the demag curve and plot_zed was classifying every record as "good" by accident.
  • In plot_zij, extend amin to include gXYZ.Y.min(). The amax line was correctly updated to include Y, but the parallel amin line in the commit ended up as a duplicate of the X/Z-only amin above it, so I think extending it to Y as well matches the intent. Also stripped a couple of #DEBUG comments left over from that session.
  • Updated the plot_zed docstring from the old 5-column signature to the actual 6-column form [step, dec, inc, M, type, quality]. The stale docstring may have been what prompted the index shift in the first place, so worth fixing at the same time.

Context

Every place in the codebase that builds a datablock and hands it to one of these functions puts the quality flag at index 5:

Where the datablock is built Datablock
programs/zeq.py [step, dec, inc, int, '', 'g']
ipmag.zeq [treatment, declination, inclination, intensity, type, quality]
ipmag.zeq_magic [treat, dec, inc, moment, blank, quality]
ipmag.plot_dmag / dmag_magic [dmag, 0, 0, int, 1, quality]
pmag.find_dmag_rec (7-col) [tr, dec, inc, int, ZI, flag, inst]
programs/demag_gui.py, programs/thellier_gui.py [tr, dec, inc, int, ZI, flag, inst]

pmag.domean also reads the quality flag from index 5, so keeping the plotting functions consistent with index 5 avoids it drifting out of step with the rest of the pipeline.

The quality flag in the datablock is at index 5, not 4 — index 4 is the
type/blank column. Every datablock constructor in the codebase
(ipmag.zeq, ipmag.zeq_magic, ipmag.plot_dmag, programs/zeq.py,
programs/demag_gui.py, pmag.find_dmag_rec, etc.) places quality at
index 5, and pmag.domean also reads it from index 5. With rec[4],
plot_mag silently excluded every point from the main demag curve, and
plot_zed's good/bad classification fell through to the else branch for
all records.

Also fix plot_zij axis scaling so amin is extended to include Y (the
previous line was a duplicate no-op of the X/Z-only amin), remove
leftover #DEBUG tags, and update the plot_zed docstring to reflect the 6-column datablock format.
@Swanson-Hysell Swanson-Hysell requested a review from apivarunas May 5, 2026 23:02
@Swanson-Hysell

Swanson-Hysell commented May 5, 2026

Copy link
Copy Markdown
Member Author

Can you have a look at this @apivarunas? I want to merge in this pull request before the next pip release as I think it fixes a bug that needs to be addressed. It would be good to do the next pip release soon due to the igrf issue you identified in #850 that could be fixed by #851

I think it should be good to go, but it would benefit from a review from you.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR corrects demagnetization plotting behavior in pmagpy/pmagplotlib.py by restoring the expected datablock quality-flag index and fixing Zijderveld axis-range calculation/documentation to match the rest of the codebase.

Changes:

  • Restores plot_mag and plot_zed to read quality flags from column 5, matching how demag datablocks are built across ipmag and programs/zeq.py.
  • Fixes plot_zij so the minimum axis extent considers Y as well as X/Z, keeping the scaling logic symmetric with the max calculation.
  • Updates the plot_zed docstring to document the actual 6-column datablock format and removes leftover debug comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@ltauxe

ltauxe commented May 6, 2026 via email

Copy link
Copy Markdown
Member

Comment thread pmagpy/pmagplotlib.py
datablock : nested list of [step, dec, inc, M (Am2), type, quality]
step : units assumed in SI
M : units assumed Am2
quality : [g,b], good or bad measurement; if bad will be marked as such

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this docstring indicate what the "type" entry in the datablock is?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call. I will add that. Something like:

datablock : nested list of [step, dec, inc, M (Am2), type, quality]
    (type indicates the IZZI step for paleointensity experiments —
    'ZI'/'IZ' or 1/0; empty string for pure demagnetization data)

@apivarunas apivarunas merged commit b2834ab into master May 6, 2026
6 checks passed
@Swanson-Hysell

Swanson-Hysell commented May 6, 2026

Copy link
Copy Markdown
Member Author

@ltauxe It does look like a bug crept in with your commit 81a4339 that I tried to address here. There are some continued patches that need to be made moving forward related to the newest Pandas version. If you could test the issue you were having now that this PR has been merged that would be great.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants