[SEDONA-238] Implement OGC GeometryType #873
Conversation
| @@ -120,6 +121,16 @@ | |||
|
|
|||
| _call_st_function = partial(call_sedona_function, "st_functions") | |||
|
|
|||
| @validate_argument_types | |||
| def GeometryType(geometry: ColumnOrName): | |||
| """Return the type of the geometry as a string. | |||
There was a problem hiding this comment.
Please add measured geometry explanation in the documentation
| @@ -1,3 +1,25 @@ | |||
| ## GeometryType | |||
|
|
|||
There was a problem hiding this comment.
Please add measured geometry explanation in the documentation along with examples
docs/api/flink/Function.md
Outdated
| @@ -1,3 +1,25 @@ | |||
| ## GeometryType | |||
|
|
|||
| Introduction: Returns the type of the geometry as a string. Eg: 'LINESTRING', 'POLYGON', 'MULTIPOINT', etc. | |||
There was a problem hiding this comment.
Please add measured geometry explanation in the documentation along with examples
| @@ -1078,6 +1078,34 @@ public void affine2DHybridGeomCollection() { | |||
| assertEquals(expectedPolygon2.toText(), actualGeomCollection.getGeometryN(0).getGeometryN(1).getGeometryN(1).toText()); | |||
| } | |||
|
|
|||
| @Test | |||
| public void geometryTypeWithMeasured() { | |||
There was a problem hiding this comment.
There is already a static object GEOMETRY_FACTORY to create geometries, there's no need to created new GeometryFactory() objects during the tests.
Please use GEOMETRY_FACTORY here
|
|
||
| public static Boolean isMeasuredGeometry(Geometry geom) { | ||
| Coordinate[] coordinates = geom.getCoordinates(); | ||
| GeometryFactory geometryFactory = new GeometryFactory(); |
There was a problem hiding this comment.
Sedona will not support creating geometryCollections with hybrid dimensions. Hence, to check for measure availability, it is better to poll 1 coordinate using getCoordinate, and check for presence of M by checking !Double.isNan(coordinate.getM()). This avoids having to create a geometryFactory, and a coordinate sequence everytime.
| String expected3 = "LINESTRINGM"; | ||
| String actual3 = Functions.geometryTypeWithMeasured(measuredLineString); | ||
| assertEquals(expected3, actual3); | ||
|
|
There was a problem hiding this comment.
Have you checked for XYZM coordinate geometries? I see that PostGIS does not implement that properly even though documentation says otherwise.
Does our function return say POINTM for POINT with XYZM coordinates? Please add testcases testing that
There was a problem hiding this comment.
Our function can return POINTM for POINT with XYZM coordinates. But it seems that PostGIS can't return that:
postgres=# SELECT GeometryType(ST_GeomFromText('POINTM(0 0 1 0)'));
ERROR: can not mix dimensionality in a geometry
HINT: "POINTM(0 0 1 0)" <-- parse error at position 20 within geometry
Should I need to add test for this file?
There was a problem hiding this comment.
Try POINT (0 0 1 0) or POINT ZM (0 0 1 0) on PostGIS, they dont throw error, but return POINT.
In our case, yes please add test cases testing XYZM
| // Create a linestring with measure value | ||
| CoordinateXYM[] coordsLineString = new CoordinateXYM[] {new CoordinateXYM(1, 2, 3), new CoordinateXYM(4, 5, 6)}; | ||
| LineString measuredLineString = new GeometryFactory().createLineString(coordsLineString); | ||
| String expected3 = "LINESTRINGM"; |
There was a problem hiding this comment.
Please add test case testing geometry collections
Did you read the Contributor Guide?
Is this PR related to a JIRA ticket?
[SEDONA-XXX] my subject.What changes were proposed in this PR?
Add ST_Dimension function to sedona-common, sedona-sql, sedona-flink, and the dataframe/python API.
How was this patch tested?
Did this PR include necessary documentation updates?