ST_HausdorffDistance function#790
Conversation
|
Thanks for the contribution! Could you please clarify the use cases you want to support by adding this st function? Also,
Also, please add the Apache license header to any new files. |
|
@zhangfengcdt thanks for the comment, I want to first ask if g1.hausdorff_distance(&g2) would work?, is hausdorff_distance already defined? |
Yes, geos already support it - https://raw.githubusercontent.com/georust/geos/master/src/geometry.rs You might want to check how this one works and follow the same pattern: |
|
@zhangfengcdt yet I cannot see the defining function, could you point out to the exact logic defining the hausdorff_distance? |
|
@zhangfengcdt why the tests are in python while the implementation in rust? |
|
@paleolimbot this function should implemented in c/sedona-geos/src or rust/sedona-functions/src? |
paleolimbot
left a comment
paleolimbot
left a comment
There was a problem hiding this comment.
Thank you for working on this!
| // under the License. | ||
| --- | ||
| title: ST_HausdorffDistance | ||
| description: Returns the Hausdorff distance between two geometries. This is a measure of how similar two geometries are. |
There was a problem hiding this comment.
| description: Returns the Hausdorff distance between two geometries. This is a measure of how similar two geometries are. | |
| description: Returns the Hausdorff distance between two geometries. |
| - returns: double | ||
| args: | ||
| - geom1: geometry | ||
| - geom2: geometry | ||
| - returns: double | ||
| args: | ||
| - geom1: geometry | ||
| - geom2: geometry | ||
| - density_frac: double |
There was a problem hiding this comment.
We have funny hand-rolled syntax for these, but this should work:
| - returns: double | |
| args: | |
| - geom1: geometry | |
| - geom2: geometry | |
| - returns: double | |
| args: | |
| - geom1: geometry | |
| - geom2: geometry | |
| - density_frac: double | |
| - returns: double | |
| args: | |
| - geometry | |
| - geometry | |
| - returns: double | |
| args: | |
| - geometry | |
| - geometry | |
| - name: density_frac | |
| type: float64 | |
| description: Densification to apply before calculation |
| (None, None, None, None), | ||
| ("POINT (0 0)", None, None, None), |
There was a problem hiding this comment.
Can you add (None, "POINT (0 1)", None, ...), ("POINT (0 1)", "POINT (0 1)", None), and checks for EMTPY behaviour (empty linestring + non empty linestring and reversed)
| query = f"SELECT ST_HausdorffDistance({geom_or_null(geom1)}, {geom_or_null(geom2)})" | ||
| else: | ||
| query = f"SELECT ST_HausdorffDistance({geom_or_null(geom1)}, {geom_or_null(geom2)}, {density_frac})" | ||
|
|
There was a problem hiding this comment.
Can you separate the tests for the version with and without density_frac (i.e., def test_st_hasdorff_distance_density_frac()? This will let us separate the "null" from "not specified" case.
There was a problem hiding this comment.
This can be added next to wherever st_distance is tested (I'm not sure it needs its own file)
| })?; | ||
|
|
||
| executor.finish(Arc::new(builder.finish())) | ||
| } |
There was a problem hiding this comment.
Can you add Rust tests (you can use the tests for st_distance as a template). The rust tests check for basic functionality and ensure that scalar and array arguments work everywhere.
3 participants
No description provided.