Kontinuation commented on code in PR #2298:
URL: https://github.com/apache/sedona/pull/2298#discussion_r2289731699


##########
common/src/main/java/org/apache/sedona/common/geography/Constructors.java:
##########
@@ -264,4 +264,241 @@ private static Geometry collectionToGeom(Geography g, 
GeometryFactory gf) {
     }
     return gf.createGeometryCollection(gs);
   }
+
+  public static Geography geomToGeography(Geometry geom) throws Exception {
+    if (geom == null) {
+      return null;
+    }
+    Geography geography;
+    if (geom instanceof Point) {
+      geography = pointToGeog((Point) geom);
+    } else if (geom instanceof MultiPoint) {
+      geography = mPointToGeog((MultiPoint) geom);
+    } else if (geom instanceof LineString) {
+      geography = lineToGeog((LineString) geom);
+    } else if (geom instanceof MultiLineString) {
+      geography = mLineToGeog((MultiLineString) geom);
+    } else if (geom instanceof Polygon) {
+      geography = polyToGeog((Polygon) geom);
+    } else if (geom instanceof MultiPolygon) {
+      geography = mPolyToGeog((MultiPolygon) geom);
+    } else if (geom instanceof GeometryCollection) {
+      geography = geomCollToGeog((GeometryCollection) geom);
+    } else {
+      throw new UnsupportedOperationException(
+          "Geometry type is not supported: " + 
geom.getClass().getSimpleName());
+    }
+    geography.setSRID(geom.getSRID());
+    return geography;
+  }
+
+  private static Geography pointToGeog(Geometry geom) throws IOException {
+    Coordinate[] pts = geom.getCoordinates();
+    if (pts.length == 0 || Double.isNaN(pts[0].x) || Double.isNaN(pts[0].y)) {
+      return new SinglePointGeography();
+    }
+    double lon = pts[0].x;
+    double lat = pts[0].y;
+
+    S2Point s2Point = S2LatLng.fromDegrees(lat, lon).toPoint();
+    S2Builder builder = new S2Builder.Builder().build();
+    S2PointVectorLayer layer = new S2PointVectorLayer();
+    builder.startLayer(layer);
+    builder.addPoint(s2Point);
+
+    S2Error error = new S2Error();
+    if (!builder.build(error)) {
+      throw new IOException("Failed to build S2 point layer: " + error.text());
+    }

Review Comment:
   We are still throwing IOExceptions where there's no IO involved. We can 
throw `IllegalArgumentException` to indicate that the input is not right.
   
   Actually, S2Error simply originates from an `IllegalArgumentException`: 
https://github.com/google/s2-geometry-library-java/blob/57151fc0f7fe543b75e9f743bf897e81928d5c3f/library/src/com/google/common/geometry/S2Builder.java#L895-L906



##########
common/src/main/java/org/apache/sedona/common/geography/Constructors.java:
##########
@@ -264,4 +264,241 @@ private static Geometry collectionToGeom(Geography g, 
GeometryFactory gf) {
     }
     return gf.createGeometryCollection(gs);
   }
+
+  public static Geography geomToGeography(Geometry geom) throws Exception {
+    if (geom == null) {
+      return null;
+    }
+    Geography geography;
+    if (geom instanceof Point) {
+      geography = pointToGeog((Point) geom);
+    } else if (geom instanceof MultiPoint) {
+      geography = mPointToGeog((MultiPoint) geom);
+    } else if (geom instanceof LineString) {
+      geography = lineToGeog((LineString) geom);
+    } else if (geom instanceof MultiLineString) {
+      geography = mLineToGeog((MultiLineString) geom);
+    } else if (geom instanceof Polygon) {
+      geography = polyToGeog((Polygon) geom);
+    } else if (geom instanceof MultiPolygon) {
+      geography = mPolyToGeog((MultiPolygon) geom);
+    } else if (geom instanceof GeometryCollection) {
+      geography = geomCollToGeog((GeometryCollection) geom);
+    } else {
+      throw new UnsupportedOperationException(
+          "Geometry type is not supported: " + 
geom.getClass().getSimpleName());
+    }
+    geography.setSRID(geom.getSRID());
+    return geography;
+  }
+
+  private static Geography pointToGeog(Geometry geom) throws IOException {
+    Coordinate[] pts = geom.getCoordinates();
+    if (pts.length == 0 || Double.isNaN(pts[0].x) || Double.isNaN(pts[0].y)) {
+      return new SinglePointGeography();
+    }
+    double lon = pts[0].x;
+    double lat = pts[0].y;
+
+    S2Point s2Point = S2LatLng.fromDegrees(lat, lon).toPoint();
+    S2Builder builder = new S2Builder.Builder().build();
+    S2PointVectorLayer layer = new S2PointVectorLayer();
+    builder.startLayer(layer);
+    builder.addPoint(s2Point);
+
+    S2Error error = new S2Error();
+    if (!builder.build(error)) {
+      throw new IOException("Failed to build S2 point layer: " + error.text());
+    }
+    List<S2Point> points = layer.getPointVector();
+    if (points.isEmpty()) {
+      return new SinglePointGeography();
+    }
+    return new SinglePointGeography(points.get(0));
+  }
+
+  private static Geography mPointToGeog(Geometry geom) throws IOException {
+    Coordinate[] pts = geom.getCoordinates();
+    if (pts.length == 0 || Double.isNaN(pts[0].x) || Double.isNaN(pts[0].y)) {
+      return new PointGeography();
+    }
+    List<S2Point> points = new ArrayList<>(pts.length);
+    // Build via S2Builder + S2PointVectorLayer
+    S2Builder builder = new S2Builder.Builder().build();
+    S2PointVectorLayer layer = new S2PointVectorLayer();
+    builder.startLayer(layer);
+    // must call build() before reading out the points
+    for (Coordinate pt : pts) {
+      double lon = pt.x;
+      double lat = pt.y;
+      S2Point s2Point = S2LatLng.fromDegrees(lat, lon).toPoint();
+      builder.addPoint(s2Point);
+    }
+    S2Error error = new S2Error();
+    if (!builder.build(error)) {
+      throw new IOException("Failed to build S2 point layer: " + error.text());
+    }
+    // Extract the resulting points
+    points.addAll(layer.getPointVector());
+    return new PointGeography(points);
+  }
+
+  private static Geography lineToGeog(Geometry geom) throws IOException {
+    LineString ls = (LineString) geom;
+
+    // Build S2 points
+    List<S2Point> pts = toS2Points(ls.getCoordinates());
+    if (pts.size() < 2) {
+      // empty or degenerate → empty single polyline
+      return new SinglePolylineGeography();
+    }
+
+    S2Builder builder = new S2Builder.Builder().build();
+    S2PolylineLayer layer = new S2PolylineLayer();
+    builder.startLayer(layer);
+
+    builder.addPolyline(new S2Polyline(pts));
+
+    S2Error error = new S2Error();
+    if (!builder.build(error)) {
+      throw new IOException("Failed to build S2 polyline: " + error.text());
+    }
+    S2Polyline s2poly = layer.getPolyline();
+    return new SinglePolylineGeography(s2poly);
+  }
+
+  private static Geography mLineToGeog(Geometry geom) throws IOException {
+    MultiLineString mls = (MultiLineString) geom;
+    List<S2Polyline> features = new ArrayList<>();
+    for (int i = 0; i < mls.getNumGeometries(); i++) {
+      LineString ls = (LineString) mls.getGeometryN(i);
+      List<S2Point> pts = toS2Points(ls.getCoordinates());
+      if (pts.size() >= 2) {
+        S2Builder builder = new S2Builder.Builder().build();
+        S2PolylineLayer layer = new S2PolylineLayer();
+        builder.startLayer(layer);
+        builder.addPolyline(new S2Polyline(pts));
+        S2Error error = new S2Error();
+        if (!builder.build(error)) {
+          throw new IOException("Failed to build S2 polyline: " + 
error.text());
+        }
+        features.add(layer.getPolyline());
+      }
+      // Empty/degenerate parts are skipped
+    }
+    return new PolylineGeography(features);
+  }
+
+  private static Geography polyToGeog(Geometry geom) throws IOException {
+    // Build S2 loops: exterior + holes
+    Polygon poly = (Polygon) geom;
+    // Construct S2 polygon (parity handles holes automatically)
+    S2Polygon s2poly = toS2Polygon(poly);
+    return new PolygonGeography(s2poly);
+  }
+
+  private static Geography mPolyToGeog(Geometry geom) throws IOException {
+    final List<S2Polygon> polys = new ArrayList<>();
+
+    MultiPolygon mp = (MultiPolygon) geom;
+    for (int i = 0; i < mp.getNumGeometries(); i++) {
+      Polygon p = (Polygon) mp.getGeometryN(i);
+      S2Polygon s2 = toS2Polygon(p);
+      if (s2 != null && !s2.isEmpty()) polys.add(s2);
+    }
+
+    return new MultiPolygonGeography(
+        Geography.GeographyKind.MULTIPOLYGON, 
Collections.unmodifiableList(polys));
+  }
+
+  private static Geography geomCollToGeog(Geometry geom) throws Exception {
+    if (!(geom instanceof GeometryCollection)) {
+      throw new IllegalArgumentException(
+          "geomCollToGeog expects GeometryCollection, got " + 
geom.getGeometryType());
+    }
+
+    List<Geography> features = new ArrayList<>();
+    GeometryCollection gc = (GeometryCollection) geom;
+    for (int i = 0; i < gc.getNumGeometries(); i++) {
+      Geometry g = gc.getGeometryN(i);
+      Geography sub = geomToGeography(g);
+      if (sub != null) features.add(sub);
+    }
+    return new GeographyCollection(features);
+  }
+
+  /** Convert JTS coordinates to S2 points; drops NaNs and consecutive 
duplicates. */
+  private static List<S2Point> toS2Points(Coordinate[] coords) throws 
IOException {
+    List<S2Point> points = new ArrayList<>(coords.length);
+    for (int i = 0; i < coords.length; i++) {
+      double lon = coords[i].x;
+      double lat = coords[i].y;
+      S2Point s2Point = S2LatLng.fromDegrees(lat, lon).toPoint();
+      points.add(s2Point);
+    }
+    return points;
+  }

Review Comment:
   I don't see the code dropping NaNs and consecutive duplicates in the loop, 
the comment or the implementation may not be correct.
   
   Should we add a test case for linestring or multi-linestring containing 
consecutive duplicates?



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