Add test coverage for write_geotiff_gpu(predictor=) and read_vrt(window=) (#1690)#1692
Merged
brendancol merged 2 commits intoMay 12, 2026
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds missing behavioural tests in xrspatial.geotiff for two previously documented-but-untested kwargs, improving regression detection for GPU predictor encoding and VRT windowed reads (issue #1690).
Changes:
- Add 23 tests covering
write_geotiff_gpu(predictor=...)across predictor modes and dtypes, including CPU/GPU parity checks. - Add behavioural tests for
read_vrt(window=...)covering clipping, mosaics, transform/coord origin shifts, and dask/CuPy backends. - Update internal sweep tracking CSV to record the newly closed coverage gaps.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
xrspatial/geotiff/tests/test_kwarg_behaviour_2026_05_12_v2.py |
New test module covering write_geotiff_gpu(predictor=...) and read_vrt(window=...) behaviours across CPU/GPU and eager/dask paths. |
.claude/sweep-test-coverage-state.csv |
Records completion of coverage sweep pass 10 for geotiff (issue #1690). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+505
to
+511
| result = read_vrt(vrt, window=(0, 0, 4, 6)) | ||
|
|
||
| assert result.shape == (4, 6) | ||
| # cols 0-3 of window are cols 0-3 of left | ||
| np.testing.assert_array_equal(result.values[:, :4], left[:, :4]) | ||
| # cols 4-5 of window are cols 0-1 of right (after seam) | ||
| np.testing.assert_array_equal(result.values[:, 4:6], right[:, :2]) |
Comment on lines
+100
to
+112
| def _da_float32(arr: np.ndarray) -> xr.DataArray: | ||
| """Wrap a 2D array with float64 y/x coords.""" | ||
| h, w = arr.shape[:2] | ||
| coords = { | ||
| 'y': np.arange(h, dtype=np.float64), | ||
| 'x': np.arange(w, dtype=np.float64), | ||
| } | ||
| if arr.ndim == 2: | ||
| return xr.DataArray(arr, dims=('y', 'x'), coords=coords) | ||
| return xr.DataArray( | ||
| arr, dims=('y', 'x', 'band'), | ||
| coords={**coords, 'band': np.arange(arr.shape[2])}, | ||
| ) |
brendancol
added a commit
to brendancol/xarray-spatial
that referenced
this pull request
May 12, 2026
…fix seam window doc Two Copilot review comments on PR xarray-contrib#1692: 1. Rename `_da_float32` -> `_da_with_float_coords` and update its docstring. The helper is used for many dtypes (uint8, uint16, int32, float64, etc.) and both 2D and 3D arrays, so the float32 name was misleading. The new name reflects what is actually float32: the y/x coordinate arrays, not the element dtype. 2. Fix `test_window_across_mosaic_seam` docstring + inline comment. The doc said the window spans "cols 2..6" but the code passes `window=(0, 0, 4, 6)` (cols 0..6) and the assertions check `left[:, :4]` + `right[:, :2]`. The test is correct; only the description was wrong. Updated to say "cols 0..6 ... seam at col 4".
…ow=) (xarray-contrib#1690) Two kwargs in xrspatial.geotiff were wired up, documented, and entirely untested. write_geotiff_gpu(predictor=True/2/3) routes through five CUDA encode kernels (_predictor_encode_kernel_u8/u16/u32/u64 plus _fp_predictor_encode_kernel) and had zero direct tests. The CPU writer side has dense coverage; the GPU side had none. Drop the encode-kernel dispatch and the output decodes to garbage. read_vrt(window=) is documented in the public API, and _vrt.read_vrt implements the full thing: window clipping, multi-source dst_rect overlap, src/dst coordinate scaling, per-band nodata, GeoTransform origin shift on coords plus attrs['transform']. The only window-related VRT test was a signature-annotation pin. 23 tests added in test_kwarg_behaviour_2026_05_12_v2.py. 11 GPU-writer predictor tests: round-trip plus on-disk Predictor tag check for predictor=True/2 on u8/u16/i32 and on 3-band RGB; predictor=3 lossless round-trip on f32 and f64; predictor=3 with int dtype raises ValueError (parity with the CPU writer); CPU vs GPU pixel-exact parity for pred=2 u16 and pred=3 f32. 12 read_vrt(window=) tests covering subregion slice, full-raster equivalence, overflow and negative-offset clamps, 2x1 mosaic seam straddle and offset past the seam, transform-attr origin shift, y/x coord half-pixel shift, window+band on a multi-band VRT, and window combined with chunks (dask), gpu=True (cupy), and gpu=True+chunks (dask+cupy). Mutation against the encode-kernel dispatch in _gpu_decode.py flipped 7 predictor tests red. Mutation against the window-clip block in _vrt.py flipped the window tests red. Test coverage gap sweep 2026-05-12 (pass 10) for the geotiff module.
…fix seam window doc Two Copilot review comments on PR xarray-contrib#1692: 1. Rename `_da_float32` -> `_da_with_float_coords` and update its docstring. The helper is used for many dtypes (uint8, uint16, int32, float64, etc.) and both 2D and 3D arrays, so the float32 name was misleading. The new name reflects what is actually float32: the y/x coordinate arrays, not the element dtype. 2. Fix `test_window_across_mosaic_seam` docstring + inline comment. The doc said the window spans "cols 2..6" but the code passes `window=(0, 0, 4, 6)` (cols 0..6) and the assertions check `left[:, :4]` + `right[:, :2]`. The test is correct; only the description was wrong. Updated to say "cols 0..6 ... seam at col 4".
brendancol
force-pushed
the
deep-sweep-test-coverage-geotiff-2026-05-12-v2
branch
from
832001f to
0dd128e
Compare
2 tasks
brendancol
added a commit
that referenced
this pull request
May 12, 2026
) PRs #1692 and #1698 raced through main with conflicting expectations for out-of-bounds window=. #1692's tests were written against the pre-#1697 contract that silently clamped windows to the VRT extent (returning a smaller array). #1698 replaced that contract with the validator-rejects-up-front behaviour from #1634 / #1669 so all three backends (local, HTTP, VRT) raise ValueError on out-of-bounds windows. Update both clamp-assertion tests to use pytest.raises(ValueError) and rename them to reflect the new contract. The "negative offsets clamp to 0" path is also gone -- negative offsets now raise.
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.
Summary
Closes #1690. Tests only, no source changes.
Two kwargs in xrspatial.geotiff are wired up, documented, and entirely untested.
write_geotiff_gpu(predictor=True/2/3)routes through five CUDA encode kernels (_predictor_encode_kernel_u8/u16/u32/u64plus_fp_predictor_encode_kernel). The CPU writer'spredictor=modes have dense coverage; the GPU side had none.read_vrt(window=)had no behaviour tests -- just a signature-annotation pin. The public API documents the kwarg and_vrt.read_vrtimplements the full thing: window clip, multi-source dst_rect overlap, src/dst coord scaling, and GeoTransform origin shift on coords plusattrs['transform'].23 tests added in
xrspatial/geotiff/tests/test_kwarg_behaviour_2026_05_12_v2.py. All pass on the local GPU host.Mutation evidence
Quick fault-injection on the source paths the new tests are meant to cover:
gpu_compress_tiles-> 7 of the 11 predictor tests flip red (the other 4 don't go through the encode kernel: the reject path and the predictor-False contrast pin)._vrt.read_vrt-> the window tests flip red.Both sources reverted before commit; no source diff lands in this PR.
Test plan
pytest xrspatial/geotiff/tests/test_kwarg_behaviour_2026_05_12_v2.py -von the GPU host -> 23 passed_gpu_decode.pyconfirmed the predictor tests catch the regression_vrt.pyconfirmed the window tests catch the regression