geotiff: cover GPU writer degenerate shapes (1x1/1xN/Nx1)#2986
Merged
Conversation
brendancol
commented
Jun 6, 2026
Contributor
Author
brendancol
left a comment
There was a problem hiding this comment.
PR Review: geotiff, cover GPU writer degenerate shapes (1x1/1xN/Nx1)
Test-only PR. No source changes, so the review is about whether the new tests are valid and have teeth, not about algorithm changes.
Blockers
None.
Suggestions
None blocking. One optional note below.
Nits
- The dask+gpu test only exercises
compression="none", while the direct-writer test covers bothnoneanddeflate. Addingdeflateto the dask+gpu parametrization would close the small asymmetry, but it is not required: the codec path is shared and already covered on the direct writer.
What looks good
- The gap is real and the scope is right. Read-side degenerate shapes already cover all four backends (
read/test_degenerate_shapes.py) and the dask streaming writer covers them (integration/test_dask_pipeline.py); the GPU write path was the only hole. - The explicit
attrs['transform']matches how the read-side degenerate fixture handles the single-element coord axis, so the two suites stay consistent. - Assertions check both shape and pixel values via
assert_array_equal, so a dropped window or a wrong-shape write would fail. I confirmed the shape mismatch raises. - Temp files include the issue number and the shape/compression id, so no collisions across parametrized cases or parallel worktrees.
- Tests use the existing
requires_gpumarker, so non-CUDA hosts skip rather than error. - The state CSV normalization to one physical line per record fixes the pre-existing union-merge newline corruption.
Checklist
- Algorithm matches reference: n/a (test-only)
- All implemented backends consistent: cupy + dask+cupy write paths now covered; numpy/dask+numpy already covered
- NaN handling correct: n/a (no nodata in fixtures)
- Edge cases covered: yes, that is the point of the PR
- Dask chunk boundaries: dask+cupy path exercised on degenerate chunks
- No premature materialization: n/a
- Benchmark: not needed
- README matrix: not needed (no new public function)
- Docstrings present: yes on every new test
brendancol
commented
Jun 6, 2026
Contributor
Author
brendancol
left a comment
There was a problem hiding this comment.
Follow-up review
The one nit from the first pass is addressed in 357b454: the dask+gpu degenerate test is now parametrized over none and deflate, matching the direct-writer test. The degenerate suite is 12 tests now (3 shapes x 2 codecs for the direct writer, 3 shapes x 2 codecs for dask+gpu), all passing on a CUDA host.
No remaining findings. Test-only change, no source touched.
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 #2984.
Adds degenerate-shape (1x1 / 1xN / Nx1) round-trip coverage for the GPU write path. The read side and the dask streaming writer already cover these shapes; the GPU writer was the remaining gap (the smallest shape elsewhere in
gpu/test_writer.pywas 2x2).test_write_geotiff_gpu_degenerate_round_trip: direct_write_geotiff_gpu, parametrized over 1x1 / 1xN / Nx1 andnone/deflatecompression.test_to_geotiff_dask_gpu_degenerate_round_trip: the dask+cupy write path viato_geotiff(gpu=True), parametrized over the three shapes.Both supply an explicit
attrs['transform']since pixel size cannot be inferred from a single-element coord axis, matching the read-side degenerate fixture. Tests use the existingrequires_gpumarker so non-CUDA hosts skip.No source changes. I checked that the GPU and dask+gpu write paths already handle these shapes before writing the tests, so this is coverage, not a fix.
Backend coverage: cupy (direct GPU writer) and dask+cupy (dask+gpu write path). The numpy eager writer and dask+numpy streaming writer already had this coverage.
Test plan:
requires_gpu.This PR also normalizes the sweep state CSV to one physical line per record, fixing pre-existing union-merge newline corruption.