Copilot commented on code in PR #2257:
URL: https://github.com/apache/sedona/pull/2257#discussion_r2264158193
##########
common/src/main/java/org/apache/sedona/common/S2Geography/Geography.java:
##########
@@ -66,7 +66,9 @@ public enum GeographyKind {
ENCODED_SHAPE_INDEX(6),
CELL_CENTER(7),
SINGLEPOINT(8),
- SINGLEPOLYLINE(9);
+ SINGLEPOLYLINE(9),
+ MULTIPOLYGON(10),
+ ;
Review Comment:
[nitpick] The trailing semicolon after the last enum value is unnecessary
and violates Java style conventions. Remove the semicolon or move it to the
previous line.
```suggestion
MULTIPOLYGON(10);
```
##########
common/src/main/java/org/apache/sedona/common/S2Geography/MultiPolygonGeography.java:
##########
@@ -21,18 +21,14 @@
import com.google.common.geometry.S2Polygon;
import java.util.Collections;
import java.util.List;
-import java.util.stream.Collectors;
public class MultiPolygonGeography extends GeographyCollection {
/**
* Wrap each raw S2Polygon in a PolygonGeography, then hand it off to
GeographyCollection to do
* the rest (including serialization).
*/
- public MultiPolygonGeography(List<S2Polygon> polygons) {
- super(
- polygons.stream()
- .map(PolygonGeography::new) // wrap each S2Polygon
- .collect(Collectors.toList())); // into a List<PolygonGeography>
+ public MultiPolygonGeography(GeographyKind kind, List<S2Polygon> polygons) {
+ super(kind, polygons);
if (polygons.isEmpty()) {
new MultiPolygonGeography();
Review Comment:
This line creates a new MultiPolygonGeography instance but doesn't assign it
to anything, making it a no-op statement. This appears to be a bug - either
remove this line or assign the result to a variable.
```suggestion
// No action needed for empty polygons list
```
##########
common/src/test/java/org/apache/sedona/common/S2Geography/WKBReaderTest.java:
##########
@@ -364,6 +364,11 @@ public void MultiPolygonTest() throws ParseException {
"0106000020E6100000020000000103000020E61000000200000005000000000000000000000000000000000000000000000000000000000000000000244000000000000024400000000000002440000000000000244000000000000000000000000000000000000000000000000005000000000000000000F03F000000000000F03F000000000000F03F0000000000002240000000000000224000000000000022400000000000002240000000000000F03F000000000000F03F000000000000F03F0103000020E6100000010000000500000000000000000022C0000000000000000000000000000022C00000000000002440000000000000F0BF0000000000002440000000000000F0BF000000000000000000000000000022C00000000000000000";
String multiPolygonWkt =
"MULTIPOLYGON(((0 0,0 10,10 10,10 0,0 0),(1 1,1 9,9 9,9 1,1 1)),((-9
0,-9 10,-1 10,-1 0,-9 0)))";
+ WKBReader reader = new WKBReader();
+ Geography geo = reader.read(WKBReader.hexToBytes(hexStr));
+ // Expect a POLYGON geometry
Review Comment:
The comment is incorrect - it says 'Expect a POLYGON geometry' but the test
expects MULTIPOLYGON. Update the comment to accurately reflect what is being
tested.
```suggestion
// Expect a MULTIPOLYGON geometry
```
--
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]
