chay0112 commented on PR #2488: URL: https://github.com/apache/sedona/pull/2488#issuecomment-3515321618
> > ``` > > def test_minimum_bounding_circle(self): > > for geom in self.geoms: > > sgpd_result = GeoSeries(geom).minimum_bounding_circle() > > gpd_result = gpd.GeoSeries(geom).minimum_bounding_circle() > > self.check_sgpd_equals_gpd(sgpd_result, gpd_result) > > ``` > > Yes! Now we're on the right track. I'll explain the code a little more just for your knowledge, `test_match_geopandas_series.py` is for directly comparing our results `GeoSeries()` with the original geopandas' results `gpd.GeoSeries`. This code effectively does exactly that: It loops through a ton of different geometries (`self.geoms`) and checks that our results (`sgpd_result`) are equal (given a tolerance) to the original geopandas results (`gpd_result`). > > > However, increasing the tolerance to 0.5 allows the test to pass. Would you prefer that I update the tolerance to 0.5 in the check_sgpd_equals_gpd method > > Nice job figuring this out. I think the best way to move forward is to add a parameter to the test function and overwrite the tolerance to use 0.5 instead just for `test_minimum_bounding_circle`. Something like this: > > ```diff > @classmethod > def check_sgpd_equals_gpd( > cls, > actual: GeoSeries, > expected: gpd.GeoSeries, > + tolerance: float = 1e-2 > ): > ... > for a, e in zip(sgpd_result, expected): > ... > cls.assert_geometry_almost_equal( > - a, e, tolerance=1e-2 # 1e-2 > + a, e, tolerance > ) > ... > ``` > > Then you can call it `self.check_sgpd_equals_gpd(sgpd_result, gpd_result, tolerance=0.5)` in your test function. This approach should still loosen the test, so it can pass, while avoiding loosening the tests for other functions. If other functions still pass with a stricter test, we don't need to loosen it for them too. The definition of that test function is [here](https://github.com/apache/sedona/blob/762d6f85004c17b3bafef73dc3bb65a3a058e38d/python/tests/geopandas/test_geopandas_base.py#L55-L76) in `test_geopandas_base.py`. Awesome, that’s one of the best explanations I’ve ever received ! -- 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]
