fix: recover polygon area from GeometryCollection intersections in grid trim - #7319
Open
dulcetberg wants to merge 1 commit into
Open
fix: recover polygon area from GeometryCollection intersections in grid trim#7319dulcetberg wants to merge 1 commit into
dulcetberg wants to merge 1 commit into
Conversation
…id trim Fixes hotosm#6638. GridService.trim_grid_to_aoi() drops a partially-overlapping tile entirely whenever its intersection with the AOI comes back as anything other than a bare Polygon/MultiPolygon. But a tile touching the AOI boundary can intersect into a GeometryCollection containing a real polygon slice plus stray Point/LineString artifacts from the boundary touch - the old code discarded the whole tile in that case, losing real area and leaving gaps where the task grid should have covered the AOI. Diagnosed by @prabinoid in the issue thread; this implements the fix they identified: for a GeometryCollection result, filter .geoms for just the Polygon/MultiPolygon members and unary_union them back into a single shape to keep using. If none of the sub-geometries carry any area (a pure boundary touch), the tile is still skipped, same as before. Verified against a constructed reproduction (two AOI polygons - one overlapping the tile's interior, one only touching its edge - unioned together) that actually produces a GeometryCollection([Polygon, LineString]) intersection with Shapely, matching the reported bug exactly: old logic drops the tile despite 1.0 unit of real overlap area; new logic recovers it. Also verified the "no polygon in the collection" and "genuinely no overlap" paths still correctly skip the tile, unchanged from before. Wasn't able to run the existing test_grid_service.py suite locally - importing the backend package pulls in its full dependency tree (mail clients etc.) beyond what this module itself needs, and BaseTestCase additionally requires a live Postgres instance neither of which were available in my environment. The verification above exercises the exact Shapely operations this change makes; deferring the full suite to CI. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Brian Bergstrom <dulcetberg@gmail.com>
|
2 tasks
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
1 participant
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
Fixes #6638.
GridService.trim_grid_to_aoi()(backend/services/grid/grid_service.py) drops a partially-overlapping tile entirely whenever its intersection with the AOI comes back as anything other than a barePolygon/MultiPolygon. A tile touching the AOI boundary can intersect into aGeometryCollectioncontaining a real polygon slice plus strayPoint/LineStringartifacts from the boundary touch — the old code discarded the whole tile in that case, losing real area and leaving gaps in task coverage, matching exactly what's reported in the issue.@prabinoid already diagnosed this in the issue thread — this PR implements the fix they identified: for a
GeometryCollectionresult, filter.geomsfor just thePolygon/MultiPolygonmembers andunary_unionthem back into a single shape to keep using. If none of the sub-geometries carry any area (a pure boundary touch), the tile is still correctly skipped, same as before.Test plan
GeometryCollection([Polygon, LineString])intersection with the tile — confirmed the old logic drops the tile despite 1.0 unit of real overlap area, and the new logic recovers it correctly.LineString) and "genuinely no overlap" (is_empty) paths still correctly skip the tile, unchanged from before.tests/backend/unit/services/grid/test_grid_service.pysuite locally — importing thebackendpackage pulls in its full dependency tree (mail clients etc.) well beyond what this module needs, andBaseTestCaseadditionally requires a live Postgres instance, neither available in my environment. The verification above exercises the exact Shapely operations this change makes; deferring the full suite to CI.🤖 Generated with Claude Code