Add ST_Relate implementation using GEOS#691
Conversation
petern48
left a comment
petern48
left a comment
There was a problem hiding this comment.
I know you're still working on this, but I figured I'd leave a bit of hints to help guide you along the way.
There was a problem hiding this comment.
It looks like you've deleted many files in the submodules, presumably accidentally. This is causing CI failures. Could you revert these deletions?
Don't be afraid to ask if you need help with figuring out how to undo this.
There was a problem hiding this comment.
let me see... and will surely get back if some help is needed
|
|
||
| executor.finish(Arc::new(builder.finish())) | ||
| } | ||
| } |
There was a problem hiding this comment.
In addition to the actual rust implementation. We're going to need to add Rust tests in this file as well as python integration tests. You can take a look at this example PR (#288) to guide you if you'd like.
There was a problem hiding this comment.
oki, let me check
There was a problem hiding this comment.
We are still missing rust and python tests. Both of these are very important to have before we merge. The example PR has examples of these as well.
The rust tests should exist in this file, starting with:
#[cfg(test)]
mod tests {The new python tests for st_relate should go in test_predicates.py (since st_relate is a predicate). Make sure to read my comment (#288 (comment)) about how to iterate on developing Python integration tests
There was a problem hiding this comment.
Thanks for the guidance! I’ll add the Rust tests in the module using #[cfg(test)] mod tests {} and implement the Python tests for st_relate in test_predicates.py, following the approach in #288.
Mehak3010
force-pushed
the
feat/st-relate-text
branch
from
6e098db to
760ac4c
Compare
Mehak3010
force-pushed
the
feat/st-relate-text
branch
from
43722df to
c6623a4
Compare
Mehak3010
force-pushed
the
feat/st-relate-text
branch
from
718eb17 to
cedfd67
Compare
Mehak3010
force-pushed
the
feat/st-relate-text
branch
from
5dad3ab to
5136544
Compare
Mehak3010
force-pushed
the
feat/st-relate-text
branch
from
357366b to
c5f171e
Compare
Mehak3010
force-pushed
the
feat/st-relate-text
branch
from
5fc9681 to
9646893
Compare
Mehak3010
force-pushed
the
feat/st-relate-text
branch
from
c867b56 to
40d256e
Compare
Mehak3010
force-pushed
the
feat/st-relate-text
branch
from
1a330ef to
4b467fc
Compare
Mehak3010
force-pushed
the
feat/st-relate-text
branch
from
be0152e to
156993f
Compare
|
Thanks for the suggestions! I have reverted the submodule changes and aligned the implementation with the structure used in PR #288 as recommended. All CI checks are now passing and the PR should be ready for review/merge. |
| crate::st_xyzm::st_y_udf, | ||
| crate::st_xyzm::st_z_udf, | ||
| crate::st_zmflag::st_zmflag_udf, | ||
| crate::st_relate::st_relate_udf, |
There was a problem hiding this comment.
nit: let's place this in alphabetical order too
|
|
||
| executor.finish(Arc::new(builder.finish())) | ||
| } | ||
| } |
There was a problem hiding this comment.
We are still missing rust and python tests. Both of these are very important to have before we merge. The example PR has examples of these as well.
The rust tests should exist in this file, starting with:
#[cfg(test)]
mod tests {The new python tests for st_relate should go in test_predicates.py (since st_relate is a predicate). Make sure to read my comment (#288 (comment)) about how to iterate on developing Python integration tests
Mehak3010
force-pushed
the
feat/st-relate-text
branch
from
bd9f15e to
64359d7
Compare
Mehak3010
force-pushed
the
feat/st-relate-text
branch
2 times, most recently
from
7f4b79d to
e178a68
Compare
3 participants
This PR implements the
ST_Relatefunction using the GEOSrelateoperation to compute the DE-9IM (Dimensionally Extended 9-Intersection Model) relationship between two geometries. The implementation integrates with the existing GEOS execution framework in Sedona and returns the resulting DE-9IM intersection matrix as a UTF-8 text string. The changes include adding the GEOS kernel implementation insedona-geos,creating the corresponding scalar UDF wrapper insedona-functions,and registering the function so it can be used through the SQL interface. This enables users to evaluate spatial relationships between geometries and obtain the DE-9IM matrix representation directly. This work addresses issue #539 and is part of the broader implementation tracked in #174.