Rasterize LinearRing as a line; warn on dropped geometries - #3063
Merged
Conversation
_classify_geometries dropped non-empty LinearRings (shapely type id 2) because the line bucket only matched LineString and MultiLineString. A LinearRing came back in all three empty buckets, so rasterize returned an all-fill raster with no error or warning. Add type id 2 (and the 'LinearRing' name on the GeometryCollection slow path) to the line bucket, and warn naming the type whenever a non-empty geometry matches no bucket. None / empty inputs stay silent. The classifier is shared by all four backends, so the fix covers numpy, cupy, dask+numpy, and dask+cupy.
brendancol
commented
Jun 9, 2026
brendancol
left a comment
Contributor
Author
There was a problem hiding this comment.
PR Review: Rasterize LinearRing as a line; warn on dropped geometries
Blockers (must fix before merge)
- None.
Suggestions (should fix, not blocking)
- None blocking. The
elsebranch in_classify_one(slow path,rasterize.py:366) is effectively unreachable through the public API, since shapely won't build a GeometryCollection that holds an unsupported member. It is reasonable as defense-in-depth, and the helper itself is unit-tested, so I would keep it.
Nits (optional improvements)
_warn_dropped_geometrieswarns with the defaultUserWarning. That matches the rest of the module (no explicit warning category is used anywhere inrasterize.py), so no change is needed. Noting it only so the choice is visible.
What looks good
- LinearRing now routes to the line bucket on both the vectorized fast path (
rasterize.py:308) and the GeometryCollection slow path (rasterize.py:363), and the tests confirm a ring burns the same pixels as the equivalent closed LineString. - The
Nonecase is handled correctly:valid = ~empty & (type_ids >= 0)(rasterize.py:300) drops type id -1, so aNonegeometry is skipped silently, matching the slow path's existinggeom is Noneguard instead of warning on it. - The drop warning fires once per batch with the unique offending types, not once per geometry, so it will not spam.
- Backend coverage is real:
_classify_geometriesis the single classifier shared by numpy, cupy, dask+numpy, and dask+cupy, and the tests exercise all four end to end.
Checklist
- Algorithm matches intended behavior (LinearRing = closed line)
- All implemented backends produce consistent results
- NaN / None / empty handling is correct
- Edge cases covered (None, empty, unsupported type id, GC slow path)
- Dask chunk boundaries handled (shared classifier; ragged chunks tested)
- No premature materialization or unnecessary copies
- Benchmark not needed (classification-only bug fix)
- README feature matrix unchanged (no new function, no backend change)
- Docstrings present and updated
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 #3055
_classify_geometriessorts inputs into polygon / line / point buckets. The line bucket matched only LineString (1) and MultiLineString (5), so a non-empty LinearRing (shapely type id 2) fell through every bucket and disappeared.rasterizethen returned an all-fill raster with no error or warning.'LinearRing'name to the slow-path (GeometryCollection) line branch, so a LinearRing rasterizes as its closed boundary.rasterizedocstring to list LinearRing and the new drop-with-warning behavior._classify_geometriesis the single classifier shared by every backend path, so the fix applies to numpy, cupy, dask+numpy, and dask+cupy.Test plan
rasterizeburns a LinearRing boundary and matches the equivalent closed LineString (numpy, dask, cupy, dask+cupy)