Add rasterize all_touched-line and non-iterable-input coverage tests#3120
Merged
Conversation
brendancol
commented
Jun 9, 2026
brendancol
left a comment
Contributor
Author
There was a problem hiding this comment.
PR Review: Add rasterize all_touched-line and non-iterable-input coverage tests
Test-only PR; reviewed the new file in full plus the surrounding rasterize source and the four prior coverage files.
Blockers (must fix before merge)
None.
Suggestions (should fix, not blocking)
- test_rasterize_coverage_2026_06_09.py:96-99:
_DIAGcrosses pixel corners, which is exactly where rasterization tie-breaking lives. The pins hold today, but a second line geometry that crosses cell interiors (not corners) would make the no-op pin less sensitive to tie-break changes that are unrelated to #3102. Cheap to add if a follow-up touches this file; not worth a new commit on its own.
Nits (optional improvements)
- test_rasterize_coverage_2026_06_09.py:185: a string input is iterable, so
rasterize("abc")takes a different failure path (item[0]on a 1-char string). The new tests cover the documented non-iterable guard only; if anyone wants the string case pinned it belongs with #3105's LOW leftovers, not this PR.
What looks good
- The no-op pin runs on all four backends with ragged dask chunks ((3,2),(3,2) over 5x5), so the tile path crosses the diagonal.
- The strict xfail referencing #3102 plus the equality pins mean the eventual fix cannot land without touching this file, which is the point of a pin.
- Bresenham expectation is a hand-checkable known value (anti-diagonal), not a snapshot.
- rasterio gating matches the convention in test_rasterize_all_touched_supercover_2169.py.
- State CSV row is a single physical line, last-write-wins, prior pass notes preserved.
Checklist
- Algorithm matches reference (rasterio default mode verified directly)
- All implemented backends produce consistent results (cupy and dask+cupy executed on a CUDA host)
- NaN handling: n/a for these paths
- Edge cases covered (corner-crossing diagonal, MultiLineString explode, ragged chunks)
- Dask chunk boundaries handled correctly
- No premature materialization (single compute per assertion)
- Benchmark not needed (test-only)
- README feature matrix: n/a
- Docstrings present on every test class and the module header
brendancol
commented
Jun 9, 2026
brendancol
left a comment
Contributor
Author
There was a problem hiding this comment.
Follow-up review (after 96d0b65)
Disposition of the first-pass findings:
- Suggestion (interior-crossing line): fixed. The no-op pin is now parametrized over a corner-crossing diagonal and a shallow interior-crossing line on all four backends (16 cells total; 15 passed + 1 strict xfail on a CUDA host).
- Nit (string input): dismissed. A string is iterable, so it takes the pair-unpacking path and dies with an incidental IndexError. Pinning that would lock in accidental behavior for garbage input rather than a contract; left with the LOW leftovers on #3105.
No new findings. The diff since the first review is the parametrize change plus the _INTERIOR constant and comment.
Contributor
Author
|
@copilot resolve the merge conflicts in this pull request |
…age-rasterize-2026-06-09 # Conflicts: # .claude/sweep-test-coverage-state.csv
Contributor
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.
Test-only PR from the test-coverage sweep (pass 5 over
xrspatial/rasterize.py).Proposed Changes
all_touched=Trueas a no-op for LineString input on numpy, cupy, dask+numpy, and dask+cupy, parametrized over a corner-crossing diagonal and a shallow interior-crossing line, plus equality with rasterio's default-mode burn. A strict xfail asserts rasterio all_touched parity and flips when rasterize(all_touched=True) has no effect on LineString geometries #3102 (lines skip the supercover burn) is fixed, so the behavior change cannot land silently.geometriesTypeError in_parse_inputover int / float / None / object inputs.main(rasterize source fixes, reproject improvements, geotiff updates); resolved conflict in.claude/sweep-test-coverage-state.csvby keeping the PR's rasterize Pass 5 row and taking main's updated reproject row.No source changes beyond the merge. The line/all_touched divergence itself is tracked in #3102.
Backend coverage: the no-op pin runs on all four backends; cupy and dask+cupy executed on a CUDA host (not skipped). Tests: 10 passed, 6 skipped.