petern48 commented on code in PR #2492:
URL: https://github.com/apache/sedona/pull/2492#discussion_r2517050289
##########
python/tests/geopandas/test_match_geopandas_series.py:
##########
@@ -833,7 +833,26 @@ def test_transform(self):
pass
def test_force_2d(self):
- pass
+ # force_2d was added from geopandas 1.0.0
+ if parse_version(gpd.__version__) < parse_version("1.0.0"):
+ pytest.skip("geopandas force_2d requires version 1.0.0 or higher")
Review Comment:
Looks like you figured it out, but I'll explain how I figure out the version
since you asked before. Usually, I'll check the geopandas changelog and see
what version the function was introduced. For example, `force_2d` was
introduced
[here](https://geopandas.org/en/stable/docs/changelog.html#:~:text=GeoSeries/GeoDataFrame%20(%233092).-,Added%20force_2d%20and,-force_3d%20methods%20from)
in the changelog, which falls under geopandas version 1.0.0. So this code is
perfect.
##########
python/tests/geopandas/test_match_geopandas_series.py:
##########
@@ -833,7 +833,26 @@ def test_transform(self):
pass
def test_force_2d(self):
- pass
+ # force_2d was added from geopandas 1.0.0
+ if parse_version(gpd.__version__) < parse_version("1.0.0"):
+ pytest.skip("geopandas force_2d requires version 1.0.0 or higher")
+ # 1) No-op on existing 2D fixtures
+ for geom in self.geoms:
+ sgpd_result = GeoSeries(geom).force_2d()
+ gpd_result = gpd.GeoSeries(geom).force_2d()
+ self.check_sgpd_equals_gpd(sgpd_result, gpd_result)
+
+ # 2) Minimal 3D sample to verify Z is actually stripped
+ data = [
+ Point(0, -1, 2.5),
+ LineString([(0, 0, 1), (1, 1, 2)]),
+ Polygon([(0, 0, 1), (1, 0, 2), (1, 1, 3), (0, 0, 1)]),
+ Point(5, 5), # already 2D
+ Polygon(), # empty geometry
+ ]
+ sgpd_3d = GeoSeries(data).force_2d()
+ gpd_3d = gpd.GeoSeries(data).force_2d()
+ self.check_sgpd_equals_gpd(sgpd_3d, gpd_3d)
Review Comment:
Nice 👍 . Even I forgot the current set of `self.geoms` doesn't include 3D
yet.
I do [hope to add them](https://github.com/apache/sedona/issues/2392),
though it could be a decent amount of effort. For now, this is great
##########
python/sedona/spark/geopandas/base.py:
##########
Review Comment:
Looks like you based this branch off of your old `minimum_bounding_circle`
branch, so all of those changes are showing up in this PR too. Can you remove
these and isolate this PR to just the `force_2d` changes?
There's probably a few ways to do this.
- Add new commits to manually delete them
- Maybe merging or rebasing might work?
- Maybe `git cherry-pick` your new force_2d changes to a new branch based
from main.
Then either force push to this PR or make a new PR.
As long as what we see in the "Files Changed" tab at the top of this page
doesn't show those changes, we should be good.
--
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]
