Fix cupy zonal_stats silently ignoring nodata_values=0 (#1227)#1228
Merged
Conversation
`_stats_cupy` used `if nodata_values:` so passing 0 (a common sentinel) fell through the else branch and kept the zero cells in. Every other backend uses `is not None` and dropped them, so the same call gave different answers depending on where it ran. Switch to `is not None`. Adds a regression test that runs the same input through all four backends and asserts the results match, plus a zonal entry in the security-sweep state file.
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
Fixes #1227.
_stats_cupyusedif nodata_values:to decide whether to apply the nodata filter. That's a truthiness check, sonodata_values=0(a common sentinel for integer rasters) fell through the else branch and zeros were never dropped. Every other backend usesif nodata_values is not None:.So
zonal_stats(..., nodata_values=0)on a cupy DataArray returned different numbers than on numpy, dask+numpy, or dask+cupy, silently.The fix
One-line change at
xrspatial/zonal.py:547:is not None, to match the rest of the module.Test coverage
Added
test_stats_nodata_values_zero_across_backends_1227. It builds zones with one zero cell per zone that should be filtered out, runsstats(..., nodata_values=0, stats_funcs=['mean', 'sum', 'count'])on all four backends, and asserts the results match.Before the fix the cupy case returns
mean=[7.5, 15.0]; after,[10.0, 20.0]like the other backends.Security sweep state
Added a
zonalentry to.claude/sweep-security-state.jsoncovering this HIGH finding and the MEDIUMs I didn't fix (int32 stride overflow in_strides, no_validate_rastercall inhypsometric_integral, no memory guard on the numpy path of_regions_numpy).Test plan
pytest xrspatial/tests/test_zonal.pyfully passes (125 tests)