Skip to content
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

[SEDONA-201] add ST_MPolyFromText and ST_MLineFromText methods #718

Conversation

gmaraswa
Copy link
Contributor

Did you read the Contributor Guide?

Is this PR related to a JIRA ticket?

What changes were proposed in this PR?

Implementation of ST_MLineFromText and ST_MPolyFromText

How was this patch tested?

Scala, Python, Java integration tests

Did this PR include necessary documentation updates?

  • Yes, I have updated the documentation update.

Copy link
Member

@jiayuasu jiayuasu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution. Please make the requested changes.


Format: `ST_MLineFromWKT (Text:string)`

Since: `1.3.0`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The version should be 1.3.1

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jiayuasu I have made all the requested changes and have successfully run the test cases. Please let me know if any more changes are required.

@@ -28,4 +28,20 @@ public static Geometry geomFromWKT(String wkt, int srid) throws ParseException {
GeometryFactory geometryFactory = new GeometryFactory(new PrecisionModel(), srid);
return new WKTReader(geometryFactory).read(wkt);
}

public static Geometry mLineFromWKT(String wkt, int srid) throws ParseException {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function names should be FromText, not FromWKT

@@ -204,5 +204,15 @@ class constructorTestScala extends TestBaseScala {
var spatialRDD2 = Adapter.toSpatialRdd(df, "geometry")
Adapter.toDf(spatialRDD2, sparkSession).show(1)
}

it("Passed ST_MLineFromWKT") {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add tests for the input with SRID

@@ -185,4 +185,19 @@ public Geometry eval(@DataTypeHint("String") String kml) throws ParseException {
return new KMLReader().read(kml);
}
}

public static class ST_MPolyFromWKT extends ScalarFunction {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please create overload functions to take care of the inputs with SRID

@@ -73,4 +73,11 @@ object st_constructors extends DataFrameAPI {

def ST_PolygonFromText(coords: Column, delimiter: Column): Column = wrapExpression[ST_PolygonFromText](coords, delimiter)
def ST_PolygonFromText(coords: String, delimiter: String): Column = wrapExpression[ST_PolygonFromText](coords, delimiter)

def ST_MPolyFromWKT(wkt: Column): Column = wrapExpression[ST_MPolyFromWKT](wkt, 0)
def ST_MPolyFromWKT(wkt: String): Column = wrapExpression[ST_MPolyFromWKT](wkt, 0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please handle the case with optional SRID

@jiayuasu
Copy link
Member

jiayuasu commented Dec 2, 2022

@gmaraswa ST_MakePolygon also uses an optional parameter. Please follow the way it handles optional parameters

@jiayuasu jiayuasu merged commit 5c0f927 into apache:master Dec 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants