You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sedona.apache.org by GitBox <gi...@apache.org> on 2022/09/10 04:23:35 UTC
[GitHub] [incubator-sedona] SW186000 opened a new pull request, #686: [SEDONA-125]Allows customized CRS in ST_Transform
SW186000 opened a new pull request, #686:
URL: https://github.com/apache/incubator-sedona/pull/686
## Did you read the Contributor Guide?
- Yes, I have read [Contributor Rules](https://sedona.apache.org/community/rule/) and [Contributor Development Guide](https://sedona.apache.org/community/develop/)
## Is this PR related to a JIRA ticket?
- Yes, the URL of the assoicated JIRA ticket is https://issues.apache.org/jira/browse/SEDONA-125. The PR name follows the format `[SEDONA-XXX] my subject`.
## What changes were proposed in this PR?
add WKT support for ST_transform
## How was this patch tested?
Scala unit test
## Did this PR include necessary documentation updates?
- Yes, I have updated the documentation update.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-sedona] jiayuasu commented on a diff in pull request #686: [SEDONA-125]Allows customized CRS in ST_Transform
Posted by GitBox <gi...@apache.org>.
jiayuasu commented on code in PR #686:
URL: https://github.com/apache/incubator-sedona/pull/686#discussion_r973758792
##########
docs/api/sql/Function.md:
##########
@@ -1234,7 +1234,8 @@ MULTIPOLYGON (((-2 -3, -3 -3, -3 3, -2 3, -2 -3)), ((3 -3, 3 3, 4 3, 4 -3, 3 -3)
Introduction:
-Transform the Spatial Reference System / Coordinate Reference System of A, from SourceCRS to TargetCRS
+Transform the Spatial Reference System / Coordinate Reference System of A, from SourceCRS to TargetCRS.
+For SourceCRS and TargetCRS, WKT format is also available.
Review Comment:
Please add "since `v1.3.1`". Please also add the same sentence to the Flink SQL doc. Thank you!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-sedona] jiayuasu commented on a diff in pull request #686: [SEDONA-125]Allows customized CRS in ST_Transform
Posted by GitBox <gi...@apache.org>.
jiayuasu commented on code in PR #686:
URL: https://github.com/apache/incubator-sedona/pull/686#discussion_r967718340
##########
common/src/main/java/org/apache/sedona/common/Functions.java:
##########
@@ -18,6 +18,7 @@
import org.apache.sedona.common.utils.GeometryGeoHashEncoder;
import org.geotools.geometry.jts.JTS;
import org.geotools.referencing.CRS;
+import org.jetbrains.annotations.Nullable;
Review Comment:
@SW186000 The test failed. Please fix the Nullable annotation from "org.jetbrains.annotations"
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-sedona] jiayuasu commented on pull request #686: [SEDONA-125]Allows customized CRS in ST_Transform
Posted by GitBox <gi...@apache.org>.
jiayuasu commented on PR #686:
URL: https://github.com/apache/incubator-sedona/pull/686#issuecomment-1242787594
@SW186000 This looks good. Can you also incorporate your change to ST_Transform in Sedona Flink?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-sedona] SW186000 commented on pull request #686: [SEDONA-125]Allows customized CRS in ST_Transform
Posted by GitBox <gi...@apache.org>.
SW186000 commented on PR #686:
URL: https://github.com/apache/incubator-sedona/pull/686#issuecomment-1272679849
@jiayuasu Sorry for late reply. I fix it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-sedona] jiayuasu commented on pull request #686: [SEDONA-125]Allows customized CRS in ST_Transform
Posted by GitBox <gi...@apache.org>.
jiayuasu commented on PR #686:
URL: https://github.com/apache/incubator-sedona/pull/686#issuecomment-1272471941
@SW186000 Could you please fix the failed CI?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-sedona] SW186000 commented on a diff in pull request #686: [SEDONA-125]Allows customized CRS in ST_Transform
Posted by GitBox <gi...@apache.org>.
SW186000 commented on code in PR #686:
URL: https://github.com/apache/incubator-sedona/pull/686#discussion_r979397856
##########
docs/api/sql/Function.md:
##########
@@ -1234,7 +1234,8 @@ MULTIPOLYGON (((-2 -3, -3 -3, -3 3, -2 3, -2 -3)), ((3 -3, 3 3, 4 3, 4 -3, 3 -3)
Introduction:
-Transform the Spatial Reference System / Coordinate Reference System of A, from SourceCRS to TargetCRS
+Transform the Spatial Reference System / Coordinate Reference System of A, from SourceCRS to TargetCRS.
+For SourceCRS and TargetCRS, WKT format is also available.
Review Comment:
I add the comments.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-sedona] SW186000 commented on a diff in pull request #686: [SEDONA-125]Allows customized CRS in ST_Transform
Posted by GitBox <gi...@apache.org>.
SW186000 commented on code in PR #686:
URL: https://github.com/apache/incubator-sedona/pull/686#discussion_r973687470
##########
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;
+ }
+ }
+
+ private static boolean checkCRSCodeFormat(String CRSCode) {
+ try{
+ CRS.decode(CRSCode);
+ }
+ catch (FactoryException ex){
+ return false;
+ }
+
+ return true;
+ }
+
+ private static boolean checkCRSWKTFormat(String CRSWKT) {
+ try{
+ CRS.parseWKT(CRSWKT);
+ }
+ catch (FactoryException ex){
+ return false;
+ }
+
+ return true;
+ }
+
+ public static Geometry transform(Geometry geometry, CoordinateReferenceSystem sourceCRS, CoordinateReferenceSystem targetCRS, boolean lenient)
+ throws FactoryException,TransformException {
+ MathTransform transform = CRS.findMathTransform(sourceCRS,targetCRS,lenient);
+ return JTS.transform(geometry, transform);
+ }
Review Comment:
Sorry for reply.
I forgot deleting this function.
i will delete it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-sedona] SW186000 commented on pull request #686: [SEDONA-125]Allows customized CRS in ST_Transform
Posted by GitBox <gi...@apache.org>.
SW186000 commented on PR #686:
URL: https://github.com/apache/incubator-sedona/pull/686#issuecomment-1242818291
Updated the PR
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-sedona] Kimahriman commented on a diff in pull request #686: [SEDONA-125]Allows customized CRS in ST_Transform
Posted by GitBox <gi...@apache.org>.
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)
+ val targetCRSString = inputExpressions(2).asString(input)
- (geometry, sourceCRS, targetCRS) match {
- case (geometry: Geometry, sourceCRS: String, targetCRS: String) =>
- val lenient = if (inputExpressions.length == 4) {
- inputExpressions(3).eval(input).asInstanceOf[Boolean]
- } else {
- false
- }
- Functions.transform(geometry, sourceCRS, targetCRS, lenient).toGenericArrayData
- case (_, _, _) => null
+ val lenient = if (inputExpressions.length == 4) {
+ inputExpressions(3).eval(input).asInstanceOf[Boolean]
+ } else {
+ false
}
+
+ (Functions transform(geometry, sourceCRSString, targetCRSString, lenient)).toGenericArrayData
Review Comment:
Nit: just use dot notation like everywhere else
```suggestion
Functions.transform(geometry, sourceCRSString, targetCRSString, lenient).toGenericArrayData
```
##########
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;
+ }
+ }
+
+ private static boolean checkCRSCodeFormat(String CRSCode) {
+ try{
+ CRS.decode(CRSCode);
+ }
+ catch (FactoryException ex){
+ return false;
+ }
+
+ return true;
+ }
+
+ private static boolean checkCRSWKTFormat(String CRSWKT) {
+ try{
+ CRS.parseWKT(CRSWKT);
+ }
+ catch (FactoryException ex){
+ return false;
+ }
+
+ return true;
+ }
+
+ public static Geometry transform(Geometry geometry, CoordinateReferenceSystem sourceCRS, CoordinateReferenceSystem targetCRS, boolean lenient)
+ throws FactoryException,TransformException {
+ MathTransform transform = CRS.findMathTransform(sourceCRS,targetCRS,lenient);
+ return JTS.transform(geometry, transform);
+ }
Review Comment:
Is this used anywhere?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-sedona] SW186000 commented on a diff in pull request #686: [SEDONA-125]Allows customized CRS in ST_Transform
Posted by GitBox <gi...@apache.org>.
SW186000 commented on code in PR #686:
URL: https://github.com/apache/incubator-sedona/pull/686#discussion_r973687528
##########
common/src/main/java/org/apache/sedona/common/Functions.java:
##########
@@ -18,6 +18,7 @@
import org.apache.sedona.common.utils.GeometryGeoHashEncoder;
import org.geotools.geometry.jts.JTS;
import org.geotools.referencing.CRS;
+import org.jetbrains.annotations.Nullable;
Review Comment:
Update the PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-sedona] jiayuasu merged pull request #686: [SEDONA-125]Allows customized CRS in ST_Transform
Posted by GitBox <gi...@apache.org>.
jiayuasu merged PR #686:
URL: https://github.com/apache/incubator-sedona/pull/686
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org