jiayuasu commented on code in PR #720:
URL: https://github.com/apache/incubator-sedona/pull/720#discussion_r1037901339


##########
common/src/main/java/org/apache/sedona/common/Functions.java:
##########
@@ -280,6 +280,19 @@ public static int nPoints(Geometry geometry) {
         return geometry.getNumPoints();
     }
 
+    public static int nDims(Geometry geometry) {
+        String str = geometry.getCoordinate().toString();

Review Comment:
   Why not directly check NaN using getX getY getZ getM from coordinate class?



##########
sql/src/test/scala/org/apache/sedona/sql/functionTestScala.scala:
##########
@@ -459,6 +459,11 @@ class functionTestScala extends TestBaseScala with 
Matchers with GeometrySample
 
     }
 
+    it("Passed ST_NDims") {

Review Comment:
   Please add more test cases for 2, 3, 4D (e.g., when Z and M present, or both 
present)



##########
docs/api/sql/Function.md:
##########
@@ -1475,3 +1475,24 @@ SELECT ST_ZMin(ST_GeomFromText('LINESTRING(1 3 4, 5 6 
7)'))
 ```
 
 Output: `4.0`
+
+## ST_NDims
+

Review Comment:
   1. Please add more elaboration and examples about dimension when the input 
has M or Z.
   2. The output should be `3` integer not `3.0` double. Please correct me if I 
am wrong.
   3. The doc should be sorted by the alphabetical order



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to