jiayuasu commented on code in PR #764:
URL: https://github.com/apache/sedona/pull/764#discussion_r1105030032
##########
common/src/test/java/org/apache/sedona/common/FunctionsTest.java:
##########
@@ -238,4 +235,153 @@ public void splitHeterogeneousGeometryCollection() {
assertNull(actualResult);
}
+
+ private static boolean intersects(Set<?> s1, Set<?> s2) {
+ Set<?> copy = new HashSet<>(s1);
+ copy.retainAll(s2);
+ return !copy.isEmpty();
+ }
+
+ @Test
+ public void getGoogleS2CellIDsPoint() {
+ Point point = GEOMETRY_FACTORY.createPoint(new Coordinate(1, 2));
+ Long[] cid = Functions.s2CellIDs(point, 30);
+ Polygon reversedPolygon = S2Utils.toJTSPolygon(new S2CellId(cid[0]));
+ // cast the cell to a rectangle, it must be able to cover the points
+ assert(reversedPolygon.contains(point));
+ }
+
+ @Test
+ public void getGoogleS2CellIDsPolygon() {
+ // polygon with holes
+ Polygon target = GEOMETRY_FACTORY.createPolygon(
+ GEOMETRY_FACTORY.createLinearRing(coordArray(0.1, 0.1, 0.5,
0.1, 1.0, 0.3, 1.0, 1.0, 0.1, 1.0, 0.1, 0.1)),
+ new LinearRing[] {
+ GEOMETRY_FACTORY.createLinearRing(coordArray(0.2, 0.2,
0.5, 0.2, 0.6, 0.7, 0.2, 0.6, 0.2, 0.2))
+ }
+ );
+ // polygon inside the hole, shouldn't intersect with the polygon
+ Polygon polygonInHole = GEOMETRY_FACTORY.createPolygon(coordArray(0.3,
0.3, 0.4, 0.3, 0.3, 0.4, 0.3, 0.3));
+ // mbr of the polygon that cover all
+ Geometry mbr = target.getEnvelope();
+ HashSet<Long> targetCells = new
HashSet<>(Arrays.asList(Functions.s2CellIDs(target, 10)));
+ HashSet<Long> inHoleCells = new
HashSet<>(Arrays.asList(Functions.s2CellIDs(polygonInHole, 10)));
+ HashSet<Long> mbrCells = new
HashSet<>(Arrays.asList(Functions.s2CellIDs(mbr, 10)));
+ assert mbrCells.containsAll(targetCells);
+ assert !intersects(targetCells, inHoleCells);
+ assert mbrCells.containsAll(targetCells);
+ }
+
+ @Test
+ public void getGoogleS2CellIDsLineString() {
+ // polygon with holes
+ LineString target = GEOMETRY_FACTORY.createLineString(coordArray(0.2,
0.2, 0.3, 0.4, 0.4, 0.6));
+ LineString crossLine =
GEOMETRY_FACTORY.createLineString(coordArray(0.4, 0.1, 0.1, 0.4));
+ // mbr of the polygon that cover all
+ Geometry mbr = target.getEnvelope();
+ // cover the target polygon, and convert cells back to polygons
+ HashSet<Long> targetCells = new
HashSet<>(Arrays.asList(Functions.s2CellIDs(target, 15)));
+ HashSet<Long> crossCells = new
HashSet<>(Arrays.asList(Functions.s2CellIDs(crossLine, 15)));
+ HashSet<Long> mbrCells = new
HashSet<>(Arrays.asList(Functions.s2CellIDs(mbr, 15)));
+ assert intersects(targetCells, crossCells);
+ assert mbrCells.containsAll(targetCells);
+ }
+
+ @Test
+ public void getGoogleS2CellIDsMultiPolygon() {
+ // polygon with holes
+ Polygon[] geoms = new Polygon[] {
+ GEOMETRY_FACTORY.createPolygon(coordArray(0.1, 0.1, 0.5, 0.1,
0.1, 0.6, 0.1, 0.1)),
+ GEOMETRY_FACTORY.createPolygon(coordArray(0.2, 0.1, 0.6, 0.3,
0.7, 0.6, 0.2, 0.5, 0.2, 0.1))
+ };
+ MultiPolygon target = GEOMETRY_FACTORY.createMultiPolygon(geoms);
+ Geometry mbr = target.getEnvelope();
+ LinearRing surroundRing =
GEOMETRY_FACTORY.createLinearRing(coordArray(0.0, 0.0, 0.8, 0.0, 0.8, 0.8, 0.0,
0.8, 0.0, 0.0));
+ System.out.println(GeomUtils.getWKT(surroundRing));
+ HashSet<Long> targetCells = new
HashSet<>(Arrays.asList(Functions.s2CellIDs(target, 10)));
+ HashSet<Long> mbrCells = new
HashSet<>(Arrays.asList(Functions.s2CellIDs(mbr, 10)));
+ HashSet<Long> surroundCells = new
HashSet<>(Arrays.asList(Functions.s2CellIDs(surroundRing, 10)));
+ assert mbrCells.containsAll(targetCells);
+ assert !intersects(targetCells, surroundCells);
+ }
+
+ @Test
+ public void getGoogleS2CellIDsMultiLineString() {
+ // polygon with holes
+ MultiLineString target = GEOMETRY_FACTORY.createMultiLineString(
+ new LineString[] {
+ GEOMETRY_FACTORY.createLineString(coordArray(0.1, 0.1,
0.2, 0.1, 0.3, 0.4, 0.5, 0.9)),
+ GEOMETRY_FACTORY.createLineString(coordArray(0.5, 0.1,
0.1, 0.5, 0.3, 0.1))
+ }
+ );
+ Geometry mbr = target.getEnvelope();
+ Point outsidePoint = GEOMETRY_FACTORY.createPoint(new Coordinate(0.3,
0.7));
Review Comment:
This variable is not used.
##########
sql/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/st_functions.scala:
##########
@@ -271,4 +271,7 @@ object st_functions extends DataFrameAPI {
def ST_ZMin(geometry: Column): Column = wrapExpression[ST_ZMin](geometry)
def ST_ZMin(geometry: String): Column = wrapExpression[ST_ZMin](geometry)
+
+ def ST_S2CellIDs(geometry: Column, level: Int): Column =
wrapExpression[ST_S2CellIDs](geometry, level)
+ def ST_S2CellIDs(geometry: String, level: Int): Column =
wrapExpression[ST_S2CellIDs](geometry, level)
Review Comment:
(1) Please add simple tests for this DataFrame style API.
(2) Please add this DataFrame style API to Sedona Python as well.
##########
docs/api/sql/Function.md:
##########
@@ -1536,3 +1536,20 @@ SELECT ST_ZMin(ST_GeomFromText('LINESTRING(1 3 4, 5 6
7)'))
```
Output: `4.0`
+
+## ST_S2CellIDs
Review Comment:
(1) All functions should be put in in alphabetical order.
(2) Is the output of this function really `4.0`?
##########
sql/src/main/scala/org/apache/sedona/sql/UDF/Catalog.scala:
##########
@@ -167,7 +167,8 @@ object Catalog {
function[RS_HTML](),
function[RS_Array](),
function[RS_Normalize](),
- function[RS_Append]()
+ function[RS_Append](),
+ function[ST_S2CellIDs]()
Review Comment:
This should be put together with other ST functions.
##########
common/src/test/java/org/apache/sedona/common/S2UtilTest.java:
##########
@@ -0,0 +1,103 @@
+/**
+ * Licensed 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
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * 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.
+ */
+package org.apache.sedona.common;
+
+import com.google.common.geometry.*;
+import org.apache.sedona.common.utils.GeomUtils;
+import org.apache.sedona.common.utils.S2Utils;
+import org.junit.Test;
+import org.locationtech.jts.geom.*;
+import org.locationtech.jts.io.ParseException;
+import org.locationtech.jts.io.WKTReader;
+
+import java.text.DecimalFormat;
+import java.util.Comparator;
+import java.util.List;
+import java.util.stream.Collectors;
+
+import static org.junit.Assert.assertEquals;
+
+public class S2UtilTest {
+ public static final GeometryFactory GEOMETRY_FACTORY = new
GeometryFactory();
+
+ private Coordinate[] coordArray(double... coordValues) {
+ Coordinate[] coords = new Coordinate[(int)(coordValues.length / 2)];
+ for (int i = 0; i < coordValues.length; i += 2) {
+ coords[(int)(i / 2)] = new Coordinate(coordValues[i],
coordValues[i+1]);
+ }
+ return coords;
+ }
+
+ @Test
+ public void toS2Point() throws ParseException {
+ Coordinate jtsCoord = new Coordinate(1, 2);
+ S2Point s2Point = S2Utils.toS2Point(jtsCoord);
+ S2LatLng latLng = new S2LatLng(s2Point);
+ assertEquals(Math.round(latLng.lngDegrees()), 1);
+ assertEquals(Math.round(latLng.latDegrees()), 2);
+ }
+
+ @Test
+ public void coverPolygon() throws ParseException {
+ Polygon polygon = (Polygon) new WKTReader().read("POLYGON ((0.5 0.5, 5
0, 6 3, 5 5, 0 5, 0.5 0.5))");
+ List<S2CellId> cellIds =
S2Utils.s2RegionToCellIDs(S2Utils.toS2Polygon(polygon), 1, 5,
Integer.MAX_VALUE);
+ assertEquals(5, cellIds.size());
+
assertEquals(cellIds.stream().max(Comparator.comparingLong(S2CellId::level)).get().level(),
5);
+ }
+
+ @Test
+ public void coverPolygonWithHole() throws ParseException {
+ Polygon polygon = (Polygon) new WKTReader().read("POLYGON((0.5 0.5,5
0,5 5,0 5,0.5 0.5), (1.5 1,4 1,4 3,1.5 1))");
+ Polygon hole = (Polygon) new WKTReader().read("POLYGON((1.5 1,4 1,4
3,1.5 1))");
+ List<S2CellId> cellIds =
S2Utils.roundCellsToSameLevel(S2Utils.s2RegionToCellIDs(S2Utils.toS2Polygon(polygon),
1, 8, Integer.MAX_VALUE-1), 8);
+ S2CellId holeCentroidCell =
S2Utils.coordinateToCellID(hole.getCentroid().getCoordinate(), 8);
+ S2CellId holeFirstVertexCell =
S2Utils.coordinateToCellID(hole.getCoordinates()[0], 8);
+ assertEquals(8,
cellIds.stream().max(Comparator.comparingLong(S2CellId::level)).get().level());
+ assert(!cellIds.contains(holeCentroidCell));
+ assert(cellIds.contains(holeFirstVertexCell));
+
+ }
+
+ @Test
+ public void coverLineString() throws ParseException {
+ LineString line = (LineString) new WKTReader().read("LINESTRING (1.5
2.45, 3.21 4)");
+ List<S2CellId> cellIds =
S2Utils.s2RegionToCellIDs(S2Utils.toS2PolyLine(line), 1, 8, Integer.MAX_VALUE);
+ cellIds.forEach(c ->
System.out.println(GeomUtils.getWKT(S2Utils.toJTSPolygon(c))));
Review Comment:
Please remove System.out.println. We want to reduce the info printed to
stdout. There are a few other places in this PR also call System.out.println,
please remove them as well.
--
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-unsubscr...@sedona.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
