Add dasymetric test coverage: metadata, 1x1, Inf weight, 3-class limiting_variable#3419
Open
brendancol wants to merge 3 commits into
Open
Conversation
…ray-contrib#3407) Pins pycnophylactic empty-valid crash with xfail-strict (xarray-contrib#3406).
brendancol
commented
Jun 20, 2026
brendancol
left a comment
Contributor
Author
There was a problem hiding this comment.
PR Review: Add dasymetric test coverage (metadata, 1x1, Inf weight, 3-class limiting_variable)
Test-only PR. No source files changed, so the correctness risk is low and the review focuses on whether the tests pin the right behaviour.
Blockers (must fix before merge)
None.
Suggestions (should fix, not blocking)
- The two xfail(strict) tests in
TestPycnophylacticEmptyValiddepend on #3406 staying open. Once #3406 is fixed the strict marker flips them red (XPASS), which is the intended signal to remove the marker. The test PR is already cross-linked from the #3406 body, so whoever fixes the bug will know to drop the markers. Informational.
Nits (optional improvements)
test_inf_weight_collapses_zoneemits aRuntimeWarning: invalid value encountered in dividefromdasymetric.py:206. The module already raises this warning in normal operation and the existing tests don't suppress it, so leaving it matches the file's style. Wrapping the call innp.errstate(invalid='ignore')would give a clean pytest run, but it's not required.TestMetadataPreservationandTestSinglePixelcover numpy and dask but not cupy. The cupy dispatch is already exercised byTestCrossBackend, so this is fine; a cupy metadata assertion would be the only marginal gain and isn't worth a GPU-only test on its own.
What looks good
- Reference values are concrete, not just "runs without crashing": 1x1 returns the full zone value, the 3-class case checks the 0/5/145 split and conservation, and the Inf case pins the exact NaN/0 collapse.
- The discovered crash bug was filed separately (#3406) and pinned with xfail(strict, raises=ValueError) rather than bundled into a source fix. That keeps the test PR honest and gives the fix a built-in regression guard.
- 1x1 dask-vs-numpy parity test covers the degenerate chunk case.
Checklist
- Algorithm matches reference: n/a (test-only)
- All implemented backends produce consistent results: existing cross-backend tests cover cupy/dask+cupy; new tests add numpy/dask metadata + 1x1 parity
- NaN handling is correct: all-NaN disaggregate asserted all-NaN; pycnophylactic crash pinned via xfail
- Edge cases covered: 1x1, Inf weight, 3-class caps, empty-valid
- Dask chunk boundaries handled: 1x1 dask parity test
- No premature materialization: n/a (test-only)
- Benchmark exists or not needed: not needed
- README feature matrix: not applicable (no new public API)
- Docstrings present and accurate: each new test class has a docstring explaining intent and issue link
…age-dasymetric-2026-06-20
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.
Closes #3407.
Test-only additions to
xrspatial/dasymetric.pycoverage, found by the test-coverage sweep. No source changes.TestMetadataPreservation: assertattrs(res/crs) and coords carry throughdisaggregateandpycnophylactic(the old metadata test only checked dims/name/shape). Covers numpy and dask.TestSinglePixel: true 1x1 raster for both functions (the existing case was a 1x2 strip). 1x1 is the degenerate kernel case forpycnophylacticwhere no neighbour shift runs.TestInfWeight: pin the current behaviour for a+infweight, which collapses a zone total to 0 (silent conservation break).TestLimitingVariableThreeClass: three-classlimiting_variablewith per-class caps. Only the 2-class break was tested before, despite the docstring describing three-class dasymetric.TestPycnophylacticEmptyValid: xfail(strict) tests pinning a crash bug the sweep found, filed as pycnophylactic raises opaque ValueError on all-NaN zones or no matching zone id #3406.pycnophylacticraisesValueError(np.nanmax on a zero-size array) when no pixel is valid for smoothing (all-NaN zones, or no zone id present invalues);disaggregatereturns all-NaN on the same input. The xfail flips red once pycnophylactic raises opaque ValueError on all-NaN zones or no matching zone id #3406 is fixed.Backends: numpy and dask+numpy run locally. cupy / dask+cupy backends for
disaggregateare already covered by the existingTestCrossBackendequivalence tests, so no new GPU variants were added.Test plan:
pytest xrspatial/tests/test_dasymetric.pyon a CUDA host: 72 passed, 2 xfailed