Drop layer/interface from the sectionate call (unbreaks #43 against sectionate 0.4.0rc2) - #44
Merged
Conversation
Sectionate is removing both arguments (hdrake/sectionate#7): the layer coordinate is a dimension of the transports it is handed, so it reaches the output on its own, and the matching interface coordinate is read off whichever grid axis registers that dimension at its "center" position -- which is where `_convergence_along_section` read the two names from before handing them straight back. No behavioral change. The layer coordinate arrives either way, whatever grid sectionate is handed -- and this call hands it `hgrid`, which has no vertical axis at all, so the `interface=` name was the only thing supplying one. It never survived the call site regardless: the interface coordinate sits on its own dimension, so extracting `[...]["conv_mass_transport"]` drops it. Nothing downstream reads it; `transform_to_lambda` takes its target and target_data from `grid._ds`. Verified rather than argued: with sectionate's branch installed, the suite is 57 passed / 0 skipped / 0 failed, the three real-CM4p25-data tests included. Reverting just this hunk against the same sectionate fails 4 tests with `TypeError: ... unexpected keyword argument 'layer'`, so the along-section path these tests cover is genuinely this call. Must not merge before hdrake/sectionate#7 reaches sectionate `master`: until then `convergent_transport` still defaults to `layer="z_l", interface="z_i"`, which would mislabel a non-depth grid's output. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
hdrake
force-pushed
the
drop-sectionate-layer-interface
branch
from
August 4, 2026 04:26
0bd0cea to
58adbd0
Compare
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
hdrake
marked this pull request as ready for review
August 4, 2026 04:32
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Downstream half of hdrake/sectionate#7, which is now merged and released as sectionate 0.4.0rc2 (PyPI, 2026-08-04). Stacked on #43 (
modernize-for-v0.7.0), where this call site lives, and targeted at it.This is not a forward-looking change any more — it repairs #43 against the sectionate its own
pyproject.tomlresolves to.sectionate >= 0.4.0rc1picks up 0.4.0rc2, which no longer haslayer/interfaceparameters at all, so_convergence_along_sectionraisesTypeError: convergent_transport() got an unexpected keyword argument 'layer'. #43's last green CI run (2026-08-02) installed 0.4.0rc1, published two days before rc2; re-run today it fails.What changed upstream
convergent_transportdropped both arguments because neither was carrying information the call did not already have:"center"position.That axis is
grid.axes["Z"]— precisely where this function read the two names from before handing them straight back:So the whole change is deleting them. Five lines, one function.
Why this is a no-op beyond unbreaking the call
utr/vtrthis call passes, independent of which grid sectionate is handed.hgrid = horizontal_grid(grid), which has no vertical axis for sectionate to read one off; and even on a grid that does, the interface coordinate sits on its own dimension, so.rename(...)["conv_mass_transport"]drops it (extracting a DataArray keeps only coordinates whose dims are a subset of its own). Theinterface=name was the only thing putting it on the returned Dataset, and nothing ever read it from there.transform_to_lambdatakes both its target and itstarget_datafromgrid._ds.Verification
Checked, not argued — #43 brings a test suite that covers this path, so this is testable in a way it was not on
main:transports.pyis byte-identical to the released 0.4.0rc2 wheel (verified bydiff). That includes the three tests that run against the real CM4p25 file, andtest_tiled_boundary_transport_obeys_divergence_theorem/test_fold_transport_methods_agree, which check the along-section transport numerically rather than just calling it.TypeErrorabove — so the tests genuinely exercise this call, and this is the hunk that fixes it.convergent_transport(grid, i_c, j_c)returns outputxr.testing.assert_identicalto the same call passinglayer=grid.axes["Z"].coords["center"], interface=grid.axes["Z"].coords["outer"].Not done: the two example notebooks were not re-executed.
ECCO_AABW_watermass_budget.ipynbis the one that touches this path (along_section=True), and its data is a 2.7 GB / 12-file Zenodo record I did not pull;MOM6_water_mass_budgets.ipynbdoes not usealong_sectionat all. Worth a run of the ECCO notebook before merging, though the suite already covers the multi-tile along-section path.Note
There is an
update-sectionate-APIbranch (1e23195 Updated sectionate API after breaking v0.4.0 changes) that predates #43 and covers the pre-modernization layout. If it also addresses this, close whichever of the two is redundant.Drafted with AI assistance (Claude Code). I have read the diff and run the tests myself.