-
Notifications
You must be signed in to change notification settings - Fork 20
Fix for empty geometries. #209
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Seems good overall but mind if I add a test? |
|
Sure! To be fair, I fear more significant changes are needed for full empty support in GI. |
Tests that npoint and nring return 0 for empty polygons, verifying the init=0 fix works correctly. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
|
Codecov only complains that |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR addresses an issue where empty Arrow geometries could not be written properly, specifically fixing a bug in the calc_extent function that would fail when processing empty geometries (e.g., empty polygons). The fix adds proper handling for empty geometries by adding an init=0 parameter to sum operations and an early return for empty geometries in extent calculations.
Key Changes:
- Added
init=0parameter to allsumoperations counting points and rings to handle empty geometry collections - Added early return in
calc_extentwhen geometry is empty to prevent reduction on empty iterators - Added test coverage for empty polygon geometries
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| test/test_primitives.jl | Added MyEmptyPolygon test struct and test cases to verify proper handling of empty polygon geometries |
| src/fallbacks.jl | Fixed sum operations to handle empty collections and added empty geometry check in calc_extent |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
asinghvi17
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I'm fixing driver combinations for GeoDataFrames, and I couldn't write empty Arrow geometries at all.
In tandem with JuliaGeo/GeoArrow.jl#6
edit: Specifically, this was an empty polygon, and calling
npoints(incalc_extent) on that is an reducing an empty iterator.