Skip to content

resample: close test-coverage gaps surfaced by deep-sweep audit#2548

Merged
brendancol merged 3 commits into
mainfrom
deep-sweep-test-coverage-resample-2026-05-27
May 28, 2026
Merged

resample: close test-coverage gaps surfaced by deep-sweep audit#2548
brendancol merged 3 commits into
mainfrom
deep-sweep-test-coverage-resample-2026-05-27

Conversation

@brendancol
Copy link
Copy Markdown
Contributor

Summary

Test-only PR closing test-coverage gaps in xrspatial.resample
surfaced by /deep-sweep test-coverage on 2026-05-27. Adds
test_resample_coverage_2026_05_27.py with 70 tests (68 passing,
2 skipped on the documented dask cubic Nx1 limitation tracked in
#2547).

Gaps closed:

  • Cat 3 HIGH: Nx1 single-column input across all four backends
    and all eight methods. Existing tests covered 1x1 and 1xN strips
    but never width=1 with multiple rows.
  • Cat 2 MEDIUM: NaN parity for cupy and dask+cupy. Existing
    parity tests used random data without NaNs, so the weight-mask
    gate and spline-prepad GPU paths had no NaN coverage.
  • Cat 3 MEDIUM: all-equal-value input across methods (zero
    variance / zero gradient).
  • Cat 5 MEDIUM: non-default dim names (lat/lon,
    latitude/longitude, channel/lat/lon 3D) propagate
    without being renamed to y/x; per-dim units attrs
    round-trip.
  • Cat 3 MEDIUM: empty raster (0 rows / 0 cols) behaviour pin.

Source untouched. Filed #2547 for the cubic-on-dask Nx1 source bug
the audit surfaced; the two skipped tests gate on that fix.

Test plan

  • pytest xrspatial/tests/test_resample_coverage_2026_05_27.py
    -> 68 passed, 2 skipped
  • pytest xrspatial/tests/test_resample.py xrspatial/tests/test_resample_coverage_2026_05_27.py -> 237
    passed, 2 skipped (no regressions in existing suite)

Adds xrspatial/tests/test_resample_coverage_2026_05_27.py with 70
tests (68 passing, 2 skipped on documented dask cubic Nx1 limitation):

- Cat 3 HIGH: Nx1 single-column input across all four backends and
  all eight methods. Existing tests covered 1x1 and 1xN strips but
  never width=1 with multi-row input.
- Cat 2 MEDIUM: NaN parity for cupy and dask+cupy. Existing
  TestCuPyParity / TestDaskCuPyParity used random data without
  NaNs, so the weight-mask gate and spline-prepad GPU paths had
  no NaN coverage.
- Cat 3 MEDIUM: all-equal-value input across methods (zero-variance
  / zero-gradient input that could expose divide-by-zero in
  weight-aware kernels).
- Cat 5 MEDIUM: non-default dim names (lat/lon, latitude/longitude,
  channel for 3D) propagate without being renamed to y/x; per-dim
  units attrs round-trip.
- Cat 3 MEDIUM: empty-raster (0 rows / 0 cols) behaviour pin.

Filed issue #2547 for the cubic-on-dask Nx1 source bug surfaced by
the audit; the two skipped tests gate on that fix landing. Source
untouched -- coverage closure only.
@github-actions github-actions Bot added the performance PR touches performance-sensitive code label May 28, 2026
@brendancol
Copy link
Copy Markdown
Contributor Author

PR Review: resample test-coverage closures

Test-only PR, 70 tests across six classes targeting the gaps the deep-sweep audit named. Read the full file in worktree; no source touched.

Blockers

None.

Suggestions

  • xrspatial/tests/test_resample_coverage_2026_05_27.py:36-48_backend_available() duplicates the runtime check the @cuda_and_cupy_available / @dask_array_available decorators already perform. The duplication is justified here (decorators apply per-class, not per-parameter), but if other _coverage_*.py siblings end up reimplementing the same boolean, a general_checks.backend_available(name) would keep it in one place.
  • xrspatial/tests/test_resample_coverage_2026_05_27.py:51-57_to_numpy() is generally useful and probably reimplemented in the zonal / polygonize coverage files. Worth a follow-up to promote it to general_checks.py.

Nits

  • xrspatial/tests/test_resample_coverage_2026_05_27.py:81 — skip message says "see audit notes" but resample: cubic on dask backends fails for arrays smaller than depth=16 #2547 is the tracking issue. Embedding the issue number in the skip string keeps it greppable once the source bug lands.
  • xrspatial/tests/test_resample_coverage_2026_05_27.py:269-278 — the comment claims the (0, 0) cell averages a 2x2 window with one NaN and three 7s to 7. That assumes the average aggregate skips NaN cells. The test passing confirms the behavior; a one-line pointer at _agg_mean / _nan_aware_* would make the assumption explicit instead of implied.
  • xrspatial/tests/test_resample_coverage_2026_05_27.py:123,132test_nx1_parity_numpy_vs_backends uses .values on the numpy reference and _to_numpy(...) on the others. The helper handles all four backends; using it for the numpy path too removes the visual inconsistency.

What looks good

  • Six classes, each with a docstring naming the audit category and the why (not just the what).
  • Skip conditions point at the tracking issue (resample: cubic on dask backends fails for arrays smaller than depth=16 #2547) for the dask cubic Nx1 limitation, so the gate is visible in CI output.
  • All four backends parametrized where appropriate; CuPy / dask gates use existing decorators.
  • Fixed RNG seeds keep NaN parity deterministic.
  • TestEmptyRasterRejected accepts (IndexError, ValueError) so a future error-message improvement stays green.
  • Filename follows the project's existing _coverage_YYYY_MM_DD pattern (polygonize, rasterize, reproject, zonal already use it).

Checklist

  • All implemented backends produce consistent results
  • NaN handling exercised across numpy / cupy / dask+cupy
  • Edge cases covered (Nx1, all-equal, empty, NaN, non-default dims)
  • Dask chunk boundaries — cubic Nx1 limitation tracked separately as resample: cubic on dask backends fails for arrays smaller than depth=16 #2547
  • No premature materialization
  • Filename and helper imports match project conventions

- skip message for dask cubic Nx1 now references tracking issue #2547
  for grep-ability when the source bug is fixed.
- clarify the NaN-aware-average comment by pointing at _agg_mean /
  _nan_aware_* in resample.py rather than implying the behavior.
- test_nx1_parity_numpy_vs_backends: use _to_numpy() for the numpy
  reference too, removing the .values vs _to_numpy() inconsistency.

No test-count or behavior changes (68 passed, 2 skipped).
@brendancol brendancol merged commit 6f29848 into main May 28, 2026
3 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

performance PR touches performance-sensitive code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant