Copilot commented on code in PR #2849:
URL: https://github.com/apache/sedona/pull/2849#discussion_r3124989583
##########
common/src/test/java/org/apache/sedona/common/Geography/FunctionTest.java:
##########
@@ -144,6 +144,27 @@ public void nPoints_polygon() throws ParseException {
assertEquals(5, Functions.nPoints(g));
}
+ @Test
+ public void asText_point() throws ParseException {
+ Geography g = Constructors.geogFromWKT("POINT (1 2)", 4326);
+ String wkt = Functions.asText(g);
+ assertNotNull(wkt);
+ assertTrue("Expected POINT, got: " + wkt, wkt.startsWith("POINT ("));
+ }
+
+ @Test
+ public void asText_polygon() throws ParseException {
+ Geography g = Constructors.geogFromWKT("POLYGON ((0 0, 1 0, 1 1, 0 1, 0
0))", 4326);
+ String wkt = Functions.asText(g);
+ assertNotNull(wkt);
+ assertTrue("Expected POLYGON, got: " + wkt, wkt.startsWith("POLYGON ("));
+ }
Review Comment:
This polygon test only checks the "POLYGON (" prefix, so it won’t catch
wrong coordinates/ring ordering. Consider parsing the returned WKT and
asserting key expected vertices (within tolerance) or comparing against an
expected normalized WKT to make the test meaningful.
##########
docs/api/sql/geography/Geography-Functions/ST_AsText.md:
##########
@@ -0,0 +1,42 @@
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+ -->
+
+# ST_AsText
+
+Introduction: Returns the Well-Known Text (WKT) representation of a geography
object.
+
+Format:
+
+`ST_AsText (A: Geography)`
+
+Return type: `String`
+
+Since: `v1.9.0`
Review Comment:
The "Since" version here (v1.9.0) is inconsistent with other Geography
function docs (e.g., ST_NPoints/ST_Distance pages currently say v1.9.1). Please
verify the correct introduction version for Geography support in ST_AsText and
align it across the geography index page and function docs to avoid confusing
users.
```suggestion
Since: `v1.9.1`
```
##########
common/src/test/java/org/apache/sedona/common/Geography/FunctionTest.java:
##########
@@ -144,6 +144,27 @@ public void nPoints_polygon() throws ParseException {
assertEquals(5, Functions.nPoints(g));
}
+ @Test
+ public void asText_point() throws ParseException {
+ Geography g = Constructors.geogFromWKT("POINT (1 2)", 4326);
+ String wkt = Functions.asText(g);
+ assertNotNull(wkt);
+ assertTrue("Expected POINT, got: " + wkt, wkt.startsWith("POINT ("));
+ }
Review Comment:
This test only asserts that the output starts with "POINT (", which doesn't
validate the coordinate values. To ensure ST_AsText is correct for Geography,
parse the returned WKT and assert the X/Y values are near the expected (1, 2)
within a tolerance (accounting for any normalization/drift).
##########
docs/api/sql/geography/Geography-Functions.md:
##########
@@ -42,6 +42,7 @@ These functions operate on geography type objects.
| Function | Return type | Description | Since |
| :--- | :--- | :--- | :--- |
| [ST_AsEWKT](Geography-Functions/ST_AsEWKT.md) | String | Return the Extended
Well-Known Text representation of a geography. | v1.8.0 |
+| [ST_AsText](Geography-Functions/ST_AsText.md) | String | Return the
Well-Known Text (WKT) representation of a geography. | v1.9.0 |
Review Comment:
The new ST_AsText entry lists "Since v1.9.0", but several linked Geography
function pages currently show "Since v1.9.1" (e.g., ST_NPoints/ST_Distance).
Please confirm the correct version for ST_AsText’s Geography support and keep
the index table consistent with the individual function docs.
##########
spark/common/src/test/scala/org/apache/sedona/sql/geography/GeographyFunctionTest.scala:
##########
@@ -87,6 +87,15 @@ class GeographyFunctionTest extends TestBaseScala {
.first()
assertEquals(3, row.getInt(0))
}
+
+ it("ST_AsText") {
+ val row = sparkSession
+ .sql("SELECT ST_AsText(ST_GeogFromWKT('POINT (1 2)', 4326)) AS wkt")
+ .first()
+ val wkt = row.getString(0)
+ // S2 round-trip may introduce sub-nanometer floating-point drift
+ assertTrue(s"Expected POINT near (1, 2), got: $wkt",
wkt.startsWith("POINT ("))
+ }
Review Comment:
The ST_AsText test only checks that the returned string starts with "POINT
(", which would still pass if the coordinates are wrong. Consider parsing the
WKT and asserting the coordinate values are within a small tolerance of (1, 2)
(or otherwise validating the numeric content), so this test actually verifies
correctness of the Geography dispatch.
--
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]
