Copilot commented on code in PR #2863:
URL: https://github.com/apache/sedona/pull/2863#discussion_r3144429788
##########
common/src/main/java/org/apache/sedona/common/utils/S2Utils.java:
##########
@@ -107,13 +107,252 @@ public static Polygon toJTSPolygon(S2CellId cellId) {
return new GeometryFactory().createPolygon(coords);
}
+ /**
+ * Convert a JTS planar geometry into an S2Region whose lat/lng projection
is guaranteed to
+ * contain the input geometry.
+ *
+ * <p>Why a buffer is needed: Sedona geometries are planar — an edge between
two vertices is a
+ * straight line in (lng, lat) space — but S2 connects the same vertices
with a great-circle arc
+ * on the sphere. The two interpretations agree at the vertices but diverge
along the edges (e.g.
+ * the great-circle arc between two points at the same northern latitude
bulges northward, leaving
+ * the parallel that would form the planar chord). If we hand the JTS
vertices to S2 directly, the
+ * spherical polygon's interior is *smaller* than the planar polygon's
interior along
+ * non-meridional edges, so the S2 covering misses thin slivers of the
original planar polygon
+ * (see GH-2857).
+ *
+ * <p>To compensate, we JTS-buffer the input by an upper bound on the
worst-case great-circle
+ * deviation before converting to S2. A side effect for {@link LineString}
inputs is that the
+ * buffer turns the line into a polygon corridor; downstream callers
therefore see cells in a thin
+ * strip around the line rather than only cells the line geometrically
traverses.
+ */
public static S2Region toS2Region(Geometry geom) throws
IllegalArgumentException {
- if (geom instanceof Polygon) {
- return S2Utils.toS2Polygon((Polygon) geom);
- } else if (geom instanceof LineString) {
- return S2Utils.toS2PolyLine((LineString) geom);
+ if (!(geom instanceof Polygon) && !(geom instanceof LineString)) {
+ throw new IllegalArgumentException(
+ "only object of Polygon, LinearRing, LineString type can be
converted to S2Region");
+ }
Review Comment:
`toS2Region` rejects anything that's not a `Polygon` or `LineString`, but
the thrown message still claims `LinearRing` is supported (both at the early
guard and the final throw). Either update the message/Javadoc to match the
actual supported types, or extend the guard to accept `LinearRing` and convert
it appropriately (e.g., via `toS2Loop`). Mismatched error messages make it
harder to debug callers that pass unsupported geometries.
--
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]
