-
Notifications
You must be signed in to change notification settings - Fork 43
feat: Add ST_Perimeter implementation based on georust/geo and benchmarks #76
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
|
@zhangfengcdt CI failed |
It looks like there are some issues in calculating the perimeter for line types. I will need to fix it in geo repo and then these tests should be fixed then. According to the PostGIS documentation and OGC standards, ST_Perimeter should return the perimeter of 2D geometries only (Polygon, MultiPolygon). |
paleolimbot
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.
Thank you!
| } | ||
|
|
||
| fn invoke_scalar(wkb: &Wkb) -> Result<f64> { | ||
| Ok(wkb.perimeter_ext(&Euclidean)) |
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.
In the GEOS version I believe peter handled the 0.0 output for non-polygon inputs here. (This works too, though!)
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 adds a new ST_Perimeter function to the sedona-geo crate, implementing perimeter calculation for geometric objects using the georust/geo library's Euclidean metric.
- Implements ST_Perimeter function using
LengthMeasurableExt::perimeter_exttrait with Euclidean distance calculation - Adds comprehensive test coverage for various geometry types including polygons, multi-polygons, polygons with holes, linestrings, and points
- Includes both Rust and Python benchmarks to measure performance against other spatial databases
Reviewed Changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| rust/sedona-geo/src/st_perimeter.rs | New implementation of ST_Perimeter function with comprehensive test suite |
| rust/sedona-geo/src/register.rs | Registers the new st_perimeter function in the scalar kernel registry |
| rust/sedona-geo/src/lib.rs | Adds st_perimeter module to the crate exports |
| rust/sedona-geo/benches/geo-functions.rs | Adds Rust benchmarks for ST_Perimeter with 10 and 500 vertex polygons |
| benchmarks/test_functions.py | Adds Python benchmarks for ST_Perimeter function testing |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
This PR adds ST_Perimeter function to sedona-geo crate.
rust/sedona-geo/src/st_perimeter.rsusingLengthMeasurableExt::perimeter_exttrait fromgeo-generic-alg with Euclidean metric
Benchmark results: