Cover GriddedRegion on a 3D grid, the path #24's tests missed - #30
Draft
hdrake wants to merge 1 commit into
Draft
Conversation
`test_dimensionality.py` was written for #24 -- `_pad_center` iterating every grid axis while padding a horizontal-only field -- and covers `connected_components` and `MaskRegions`. Both stay inside `regionate.boundaries`, so neither reaches `GriddedRegion`, which traces its boundary with `sectionate.grid_section`. That is the gap that let the *same* bug survive in sectionate: `sectionate.gridutils` requested a halo on every registered axis in exactly the way `_pad_center` used to, so a 3D grid raised the same KeyError from `grid_section` (MOM6-community/sectionate#52). One bug between two libraries, fixed in only one of them. Add the missing case: build a GriddedRegion from lons/lats on both the 2D grid and its 3D counterpart and require identical indices, coordinates and mask. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Adds the one case
test_dimensionality.pywas missing. Blocked on MOM6-community/sectionate#52 — it will fail CI until that fix lands. Draft until then.Why
This file was written for #24:
regionate.boundaries._pad_centeriterated every axis of the grid while padding a field carrying only the horizontal tracer dims, so a 2-D mask on a 3-D grid raisedThat was fixed here, with the two tests in this file. Both go through
connected_componentsandMaskRegions, which stay insideregionate.boundaries. Neither builds aGriddedRegion— and that is the path that callssectionate.grid_section, and sosectionate.gridutils.build_neighbor_maps.build_neighbor_mapshad the identical bug:padding_width = {ax: (1, 1) for ax in grid.axes}on a horizontal-only index array, at three sites. So the same defect existed in both libraries, was diagnosed and fixed in this one, and survived untouched in the sibling — because the regression test stopped one function short of the boundary between them. I confirmed the existing two tests pass unchanged against stock sectionate 0.4.0rc1.Coverage counts, for context: across regionate's tests and examples there are 15
xgcm.Gridconstructions and this file holds the only one that registers aZaxis; across sectionate's 33, none do.The test
test_gridded_region_ignores_unused_z_axisbuilds aGriddedRegionfrom the same lons/lats oninitialize_spherical_grid()and on its_add_z_axis(...)counterpart, and requires identicali_c,j_c,lons_c,lats_cand mask — the same "an unused vertical axis changes nothing about horizontal topology" contract the existing tests assert, extended to the entry point that crosses into sectionate.Status
test_gridded_region_ignores_unused_z_axisKeyError)The 10 skips are the opt-in real-data tests (
REGIONATE_REALDATA_TESTS=1), unrelated to this change.CI installs sectionate from
hdrake/sectionate@topology-driven-neighbors, which does not yet carry the fix — it is onfix-horizontal-only-padding(hdrake/sectionate#8). This should merge only after that one, which is why it is a draft. It is deliberately a cross-library contract test: its job is to fail if sectionate ever regresses this, which is precisely what nothing caught the first time.Drafted with AI assistance (Claude Code). I have read the diff and ran the tests myself.