feat(c/sedona-geos): Adds ST_Normalize(geometry) in c/sedona-geos using GEOS#802
Conversation
feat(c/sedona-geos): remove boilerplate created from st_area add first iteration of ST_Normalize kernel and tests feat(c/sedona-geos): align ST_Normalize implementation with existing GEOS UDFs feat(c/sedona-geos): update st_normalize to use Clone trait feat(c/sedona-geos): add python integration tests feat(c/sedona-geos): add additional tests for ST_Normalize feat(c/sedona-geos): add additional tests for st_normalize feat(c/sedona-geos): fix accidental overwrite on numgeometries feat(c/sedona-geos): add documentation for st_normalize
paleolimbot
left a comment
paleolimbot
left a comment
There was a problem hiding this comment.
Thank you!
It's worth seeing if the slightly more compact version of the test works (in theory you should be able to put WKT in the expected slot but there are some quirks).
| if isinstance(eng, PostGIS) and expected is not None: | ||
| # Normalize expected WKT to PostGIS's compact ST_AsText formatting. | ||
| expected = expected.replace(", ", ",") | ||
| expected = expected.replace(" (", "(") | ||
| expected = expected.replace(r"ZM(", r"ZM (") | ||
| expected = expected.replace(r"M(", r"M (") | ||
| expected = expected.replace(r"Z(", r"Z (") | ||
|
|
||
| if isinstance(eng, SedonaDB) and expected is not None: | ||
| expected = expected.replace(", ", ",") | ||
| expected = expected.replace(" (", "(") | ||
|
|
||
| eng.assert_query_result( | ||
| f"SELECT ST_AsText(ST_Normalize({geom_or_null(geom)}))", expected | ||
| ) |
There was a problem hiding this comment.
This should work:
| if isinstance(eng, PostGIS) and expected is not None: | |
| # Normalize expected WKT to PostGIS's compact ST_AsText formatting. | |
| expected = expected.replace(", ", ",") | |
| expected = expected.replace(" (", "(") | |
| expected = expected.replace(r"ZM(", r"ZM (") | |
| expected = expected.replace(r"M(", r"M (") | |
| expected = expected.replace(r"Z(", r"Z (") | |
| if isinstance(eng, SedonaDB) and expected is not None: | |
| expected = expected.replace(", ", ",") | |
| expected = expected.replace(" (", "(") | |
| eng.assert_query_result( | |
| f"SELECT ST_AsText(ST_Normalize({geom_or_null(geom)}))", expected | |
| ) | |
| eng.assert_query_result( | |
| f"SELECT ST_Normalize({geom_or_null(geom)})", expected | |
| ) |
There was a problem hiding this comment.
Thank you for reviewing this, I really appreciate it! I tried the compact version, but it led to formatting-only failures in the Python harness, for example one of the diffs I ran into is:
E AssertionError: Expected:
E [('POLYGON Z ((0 0 5,0 1 5,1 1 5,1 0 5,0 0 5))',)]
E Got:
E [('POLYGON Z ((0 0 5, 0 1 5, 1 1 5, 1 0 5, 0 0 5))',)]
Without using ST_AsText, the failures seem to come from string formatting in the test harness rather than from ST_Normalize behavior itself.
There was a problem hiding this comment.
Got it! I will open a follow up to see if we can improve this at some point since at least in theory the testing module is supposed to deal with that 😬
| if isinstance(eng, PostGIS) and expected is not None: | ||
| # Normalize expected WKT to PostGIS's compact ST_AsText formatting. | ||
| expected = expected.replace(", ", ",") | ||
| expected = expected.replace(" (", "(") | ||
| expected = expected.replace(r"ZM(", r"ZM (") | ||
| expected = expected.replace(r"M(", r"M (") | ||
| expected = expected.replace(r"Z(", r"Z (") | ||
|
|
||
| if isinstance(eng, SedonaDB) and expected is not None: | ||
| expected = expected.replace(", ", ",") | ||
| expected = expected.replace(" (", "(") | ||
|
|
||
| eng.assert_query_result( | ||
| f"SELECT ST_AsText(ST_Normalize({geom_or_null(geom)}))", expected | ||
| ) |
There was a problem hiding this comment.
Got it! I will open a follow up to see if we can improve this at some point since at least in theory the testing module is supposed to deal with that 😬
2 participants
Pull Request: Implement ST_Normalize
Description
This PR introduces the
ST_Normalizefunction, leveraging the GEOS library.ST_Normalizereturns a geometry in its normalized (canonical) form, ensuring consistent representation across different processing steps.Key Changes
Tracking
This work is part of the following development tracks: