petern48 commented on code in PR #2395:
URL: https://github.com/apache/sedona/pull/2395#discussion_r2427688611
##########
python/tests/geopandas/test_match_geopandas_series.py:
##########
@@ -564,6 +564,33 @@ def test_difference(self):
)
self.check_sgpd_equals_gpd(sgpd_result, gpd_result)
+ def test_symmetric_difference(self):
+ for geom, geom2 in self.pairs:
+ # Sedona doesn't support difference for GeometryCollections
+ if isinstance(geom[0], GeometryCollection) or isinstance(
+ geom2[0], GeometryCollection
+ ):
+ continue
+ # Operation doesn't work on invalid geometries
+ if (
+ not gpd.GeoSeries(geom).is_valid.all()
+ or not gpd.GeoSeries(geom2).is_valid.all()
+ ):
+ continue
Review Comment:
(This one is just for your knowledge) In general, we want all cases to pass.
This condition here is a case that we want to keep because it raises the
following error, making our test fail.
```
E shapely.errors.GEOSException: TopologyException: side location
conflict at 1.5 0.5. This can occur if the input geometry is invalid.
```
This also means that if you implement a function test in the future that
fails for odd reasons (unrelated to the Python Sedona geopandas code), you can
write a conditional skip like this to avoid it, too. If you're unsure whether
it's reasonable, you're always welcome to ask for help.
##########
python/tests/geopandas/test_match_geopandas_series.py:
##########
@@ -564,6 +564,33 @@ def test_difference(self):
)
self.check_sgpd_equals_gpd(sgpd_result, gpd_result)
+ def test_symmetric_difference(self):
+ for geom, geom2 in self.pairs:
+ # Sedona doesn't support difference for GeometryCollections
+ if isinstance(geom[0], GeometryCollection) or isinstance(
+ geom2[0], GeometryCollection
+ ):
+ continue
Review Comment:
```suggestion
```
We can delete this. (I tried it locally, and it passes). The reason that
`difference` had this skip is because there's a bug in the Java code that
throws an error for Geom Collection. Fixing it was out-of-scope for a geopandas
PR. In this case, we can remove this because symmetric_difference doesn't have
this problem. I ran it myself locally and it passed.
--
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]
