You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sedona.apache.org by "furqaankhan (via GitHub)" <gi...@apache.org> on 2024/03/12 12:03:53 UTC

[PR] [SEDONA-480] Implement ST_S2ToGeom [sedona]

furqaankhan opened a new pull request, #1277:
URL: https://github.com/apache/sedona/pull/1277

   
   ## Did you read the Contributor Guide?
   
   - Yes, I have read [Contributor Rules](https://sedona.apache.org/latest-snapshot/community/rule/) and [Contributor Development Guide](https://sedona.apache.org/latest-snapshot/community/develop/)
   
   ## Is this PR related to a JIRA ticket?
   
   - Yes, the URL of the associated JIRA ticket is https://issues.apache.org/jira/browse/SEDONA-480. The PR name follows the format `[SEDONA-XXX] my subject`.
   
   
   ## What changes were proposed in this PR?
   
   - Expose the ST_S2ToGeom to public APIs.
   
   ## How was this patch tested?
   
   - Passed new and old tests.
   
   ## 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


Re: [PR] [SEDONA-480] Implement ST_S2ToGeom [sedona]

Posted by "jiayuasu (via GitHub)" <gi...@apache.org>.
jiayuasu merged PR #1277:
URL: https://github.com/apache/sedona/pull/1277


-- 
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


Re: [PR] [SEDONA-480] Implement ST_S2ToGeom [sedona]

Posted by "jiayuasu (via GitHub)" <gi...@apache.org>.
jiayuasu commented on code in PR #1277:
URL: https://github.com/apache/sedona/pull/1277#discussion_r1522636765


##########
common/src/test/java/org/apache/sedona/common/FunctionsTest.java:
##########
@@ -561,6 +561,27 @@ public void getGoogleS2CellIDsAllSameLevel() {
         assertEquals(expects, levels);
     }
 
+    @Test
+    public void testS2ToGeom() throws ParseException {
+        Geometry target = GEOMETRY_FACTORY.createPolygon(

Review Comment:
   Can you clean up all these comments and println?



##########
spark/common/src/test/scala/org/apache/sedona/sql/functions/STS2CellIDs.scala:
##########
@@ -100,4 +100,17 @@ class STS2CellIDs extends TestBaseScala with Matchers with GeometrySample with G
       )
     }
   }
+
+  describe("should pass ST_S2ToGeom") {
+    it("should intersect with the target geometry") {

Review Comment:
   Please add one more test to show that the result of ST_S2ToGeom can be used in ST_Collect to get a `MULTIPOLYGON`



##########
snowflake-tester/src/test/java/org/apache/sedona/snowflake/snowsql/TestFunctions.java:
##########
@@ -752,6 +752,7 @@ public void test_ST_S2CellIDs() {
                 "[\n  1153031455769624576,\n  1152961087025446912,\n  1152925902653358080,\n  1152934698746380288,\n  1152943494839402496,\n  1152952290932424704,\n  1152969883118469120,\n  1152978679211491328,\n  1152987475304513536,\n  1152996271397535744,\n  1153005067490557952,\n  1153049047955668992,\n  1153057844048691200,\n  1153040251862646784,\n  1153084232327757824,\n  1153093028420780032,\n  1153066640141713408,\n  1153075436234735616,\n  1153101824513802240,\n  1153137008885891072,\n  1153189785444024320,\n  1153198581537046528,\n  1153172193257979904,\n  1153180989351002112,\n  1153163397164957696,\n  1153128212792868864,\n  1153013863583580160,\n  1153022659676602368,\n  1153242562002157568,\n  1153216173723090944,\n  1153277746374246400,\n  1153207377630068736,\n  1153224969816113152,\n  1153233765909135360,\n  1153268950281224192,\n  1153321726839357440,\n  1153365707304468480,\n  1153374503397490688,\n  1153400891676557312,\n  1153409687769579520,\n  115338329949051
 2896,\n  1153392095583535104,\n  1153436076048646144,\n  1153444872141668352,\n  1153418483862601728,\n  1153427279955623936,\n  1153453668234690560,\n  1153462464327712768,\n  1153330522932379648,\n  1921361385166471168,\n  1921475734375759872,\n  1921484530468782080,\n  1921493326561804288,\n  1921519714840870912,\n  1921528510933893120,\n  1921537307026915328,\n  384305702186778624,\n  1152389340979003392,\n  1152398137072025600,\n  1152406933165047808,\n  1152873126095224832,\n  1152881922188247040,\n  1152890718281269248,\n  1152917106560335872\n]"
         );
     }
+

Review Comment:
   Please remove this change



-- 
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: issues-unsubscribe@sedona.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [SEDONA-480] Implement ST_S2ToGeom [sedona]

Posted by "furqaankhan (via GitHub)" <gi...@apache.org>.
furqaankhan commented on code in PR #1277:
URL: https://github.com/apache/sedona/pull/1277#discussion_r1522654291


##########
common/src/test/java/org/apache/sedona/common/FunctionsTest.java:
##########
@@ -561,6 +561,27 @@ public void getGoogleS2CellIDsAllSameLevel() {
         assertEquals(expects, levels);
     }
 
+    @Test
+    public void testS2ToGeom() throws ParseException {
+        Geometry target = GEOMETRY_FACTORY.createPolygon(

Review Comment:
   My bad, I forgot about them.



-- 
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: issues-unsubscribe@sedona.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org