Website

[Felix Cheung]

Please take a look at the bottom of this report
https://incubator.apache.org/clutch/sedona.html

And esp at
https://whimsy.apache.org/pods/project/sedona

[GitHub] [incubator-sedona] Kimahriman commented on a diff in pull request #686: [SEDONA-125]Allows customized CRS in ST_Transform

[GitBox]

Kimahriman commented on code in PR #686:
URL: https://github.com/apache/incubator-sedona/pull/686#discussion_r967808456


##
common/src/main/java/org/apache/sedona/common/Functions.java:
##
@@ -142,12 +143,53 @@ public static Geometry transform(Geometry geometry, 
String sourceCRS, String tar
 
 public static Geometry transform(Geometry geometry, String sourceCRS, 
String targetCRS, boolean lenient)
 throws FactoryException, TransformException {
-CoordinateReferenceSystem sourceCRScode = CRS.decode(sourceCRS);
-CoordinateReferenceSystem targetCRScode = CRS.decode(targetCRS);
+
+CoordinateReferenceSystem sourceCRScode = parseCRSString(sourceCRS);
+CoordinateReferenceSystem targetCRScode = parseCRSString(targetCRS);
 MathTransform transform = CRS.findMathTransform(sourceCRScode, 
targetCRScode, lenient);
 return JTS.transform(geometry, transform);
 }
 
+private static CoordinateReferenceSystem parseCRSString(String CRSString) 
throws FactoryException{
+if (checkCRSCodeFormat(CRSString) ){
+return CRS.decode(CRSString);
+}
+else if (checkCRSWKTFormat(CRSString)){
+return CRS.parseWKT(CRSString);
+}

Review Comment:
   Since these check methods just try to do the decoding, should this just be 
something like
   ```suggestion
   try {
   return CRS.decode(CRSString);
   } catch (FactoryException ex) {
   try {
   return CRS.parseWKT(CRSString);
   }  catch (FactoryException ex2) {
   // ...raise some exception maybe from one or the other or 
some custom thing explaining can't be parsed
   }
   ```



##
flink/src/main/java/org/apache/sedona/flink/expressions/Functions.java:
##
@@ -115,6 +115,14 @@ public Geometry eval(@DataTypeHint(value = "RAW", 
bridgedTo = org.locationtech.j
 Geometry geom = (Geometry) o;
 return org.apache.sedona.common.Functions.transform(geom, 
sourceCRS, targetCRS);
 }
+
+@DataTypeHint(value = "RAW", bridgedTo = 
org.locationtech.jts.geom.Geometry.class)
+public Geometry eval(@DataTypeHint(value = "RAW", bridgedTo = 
org.locationtech.jts.geom.Geometry.class) Object o, @DataTypeHint("String") 
String sourceCRS, @DataTypeHint("String") String 
targetCRS,@DataTypeHint("Boolean") Boolean lenient)
+throws FactoryException, TransformException {
+Geometry geom = (Geometry) o;
+return org.apache.sedona.common.Functions.transform(geom, 
sourceCRS, targetCRS,lenient);
+}

Review Comment:
   Nit: spacing
   ```suggestion
   public Geometry eval(@DataTypeHint(value = "RAW", bridgedTo = 
org.locationtech.jts.geom.Geometry.class) Object o, @DataTypeHint("String") 
String sourceCRS, @DataTypeHint("String") String targetCRS, 
@DataTypeHint("Boolean") Boolean lenient)
   throws FactoryException, TransformException {
   Geometry geom = (Geometry) o;
   return org.apache.sedona.common.Functions.transform(geom, 
sourceCRS, targetCRS, lenient);
   }
   ```



##
common/src/main/java/org/apache/sedona/common/Functions.java:
##
@@ -142,12 +143,53 @@ public static Geometry transform(Geometry geometry, 
String sourceCRS, String tar
 
 public static Geometry transform(Geometry geometry, String sourceCRS, 
String targetCRS, boolean lenient)
 throws FactoryException, TransformException {
-CoordinateReferenceSystem sourceCRScode = CRS.decode(sourceCRS);
-CoordinateReferenceSystem targetCRScode = CRS.decode(targetCRS);
+
+CoordinateReferenceSystem sourceCRScode = parseCRSString(sourceCRS);
+CoordinateReferenceSystem targetCRScode = parseCRSString(targetCRS);
 MathTransform transform = CRS.findMathTransform(sourceCRScode, 
targetCRScode, lenient);
 return JTS.transform(geometry, transform);
 }
 
+private static CoordinateReferenceSystem parseCRSString(String CRSString) 
throws FactoryException{
+if (checkCRSCodeFormat(CRSString) ){
+return CRS.decode(CRSString);
+}
+else if (checkCRSWKTFormat(CRSString)){
+return CRS.parseWKT(CRSString);
+}
+else {
+return null;
+}

Review Comment:
   Is this going to cause NPEs instead of throwing an exception explaining the 
string can't be parsed as a CRS?



##
sql/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/Functions.scala:
##
@@ -202,19 +206,17 @@ case class ST_Transform(inputExpressions: Seq[Expression])
 
   override def eval(input: InternalRow): Any = {
 val geometry = inputExpressions(0).toGeometry(input)
-val sourceCRS = inputExpressions(1).asString(input)
-val targetCRS = inputExpressions(2).asString(input)
+val sourceCRSString = inputExpressions(1).asString(input)
+