zonal: rename crop(zones_ids=) -> crop(zone_ids=) with deprecation shim (#2521)#2527
Merged
brendancol merged 1 commit intoMay 27, 2026
Conversation
…im (xarray-contrib#2521) stats() and crosstab() use zone_ids, but crop() historically used zones_ids (extra 's'). Switching between sibling zonal functions raised TypeError on the typo. Fix accepts both kwargs, emits DeprecationWarning on the legacy zones_ids, raises if both are passed, and raises if neither is supplied. Internal call sites migrated to the canonical zone_ids; new regression tests cover both paths plus error cases.
brendancol
added a commit
that referenced
this pull request
May 28, 2026
brendancol
added a commit
that referenced
this pull request
May 28, 2026
brendancol
added a commit
that referenced
this pull request
May 28, 2026
…m/crop/apply (#2530) * test_zonal: close HIGH backend-coverage gaps for crosstab / regions / trim / crop / apply Adds test_zonal_backend_coverage_2026_05_27.py (32 tests, all passing on a CUDA host) covering test-coverage gaps surfaced by the deep-sweep test-coverage pass on the zonal module. Cat 1 HIGH backend coverage: - crosstab cupy + dask+cupy parity vs numpy (_crosstab_cupy / _crosstab_dask_cupy were registered in ArrayTypeFunctionMapping but never invoked by any test). - regions cupy + dask+cupy parity vs numpy (cupyx.scipy.ndimage label branch + dask+cupy compute-then-label branch). - trim dask+numpy + cupy + dask+cupy parity (the cupy data.get() path and the dask _trim_bounds_dask isnan branch were untested). - crop dask+numpy + cupy + dask+cupy parity (matching _crop_bounds_dask branch). - apply 3D cupy + dask+cupy parity (per-layer kernel launch over the third axis; existing 3D apply tests covered numpy + dask+numpy only). Cat 3 MEDIUM geometric edges: - 1x1 single-pixel raster on trim / crop. - 1xN and Nx1 strips on regions. Cat 4 LOW parameter coverage: - regions(neighborhood=6) -> ValueError pin. - suggest_zonal_canvas(crs='Geographic') aspect-ratio pin and invalid-crs KeyError. - crosstab cupy with zone_ids / cat_ids subset. - crosstab cupy with agg='percentage'. Cat 5 MEDIUM metadata propagation: - regions preserves coords + attrs (numpy + dask+numpy). - trim / crop set name and preserve attrs. Also pins the documented numpy-vs-dask trim asymmetry on the NaN sentinel: numpy _trim uses equality (never matches NaN), dask _trim_bounds_dask has a dedicated isnan branch. Mutation against cupy.asnumpy() in _crosstab_cupy flipped test_crosstab_cupy_matches_numpy red, confirming the test detects regressions. Source untouched. * test_zonal_backend_coverage: apply review-pr suggestions - Lift duplicated _canonical_labels helper to module scope so a fix lands once instead of twice (was duplicated in test_regions_cupy_matches_numpy and test_regions_dask_cupy_matches_numpy). - Import cuda_and_cupy_available and dask_array_available from general_checks instead of redeclaring locally. dask_required is now a single-line alias of the shared decorator. - Drop unused imports (zonal.stats, has_cuda_and_cupy) flagged by flake8. - Tighten test_suggest_zonal_canvas_geographic_crs docstring (removed an unedited mid-sentence "wait..." correction). zones_ids=... parameter rename to zone_ids=... is intentionally deferred to rebase: this branch is behind origin/main, the rename + deprecation shim only exist on main, so renaming here would break the tests on this branch. 32 tests still pass on the CUDA host. * test_zonal_backend_coverage: use canonical crop(zone_ids=) after #2527
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.
Summary
xrspatial.zonal.crop()accepts the canonicalzone_idskwarg, matchingstats()andcrosstab().zones_idskwarg (extras) is kept as a deprecated alias; passing it now emitsDeprecationWarning.TypeError; passing neither also raisesTypeError.zone_ids; regression tests cover both paths plus the error cases.Closes #2521.
Why
The api-consistency sweep against
zonal(2026-05-27) flagged this as a HIGH Cat 1 finding: same concept named differently across sibling public functions inside one module. A user who learnedzone_idsfromstats()orcrosstab()would hitTypeError: crop() got an unexpected keyword argument 'zone_ids'and vice versa.Test Plan
pytest xrspatial/tests/test_zonal.py— all 130 tests passpytest xrspatial/tests/test_dask_cupy_gaps.py -k crop— 4 passpytest xrspatial/tests/test_validation.py -k crop— 1 pass