[GH-2504] Geopandas: Implement force_3d #2512

chay0112 · 2025-11-20T04:09:26Z

Did you read the Contributor Guide?

Yes, I have read the Contributor Rules and Contributor Development Guide

Is this PR related to a ticket?

Yes, and the PR name follows the format [GH-XXX] my subject. Closes Geopandas: Implement force_3d #2504

What changes were proposed in this PR?

Implement force_3d

How was this patch tested?

Included unit and parity tests

Did this PR include necessary documentation updates?

Yes, I have updated the documentation.

petern48

Looks good. Just some minor feedback about improving the docs and the tests. Should be good to merge afterwards.

petern48 · 2025-11-20T06:14:41Z

python/sedona/spark/geopandas/base.py

+        >>> from shapely import Polygon, LineString, Point
+        >>> s = geopandas.GeoSeries(


Suggested change

>>> from shapely import Polygon, LineString, Point

>>> s = geopandas.GeoSeries(

>>> from shapely import Polygon, LineString, Point

>>> from sedona.spark.geopandas import GeoSeries

>>> s = GeoSeries(

nit

petern48 · 2025-11-20T06:19:21Z

python/sedona/spark/geopandas/base.py

+        2D geometries will get the provided Z coordinate; 3D geometries
+        are unchanged (unless their Z coordinate is ``np.nan``).


Can you add a test for this in test_geoseries.py? I see that this is from the original geopandas docstring, so I'm not entirely sure if we follow the same behavior naturally in Sedona. If we don't let me know, and we can figure out what to do next.

We could test something like whether Point(1, 1, np.nan) becomes Point(1, 1, 3) when calling .force_3d(3)

petern48 · 2025-11-20T06:24:41Z

python/sedona/spark/geopandas/base.py

+        Note that for empty geometries, 3D is only supported since GEOS 3.9 and then
+        still only for simple geometries (non-collections).
+


Suggested change

Note that for empty geometries, 3D is only supported since GEOS 3.9 and then

still only for simple geometries (non-collections).

I think we can delete this part. The GEOS 3.9 part doesn't apply to use because we're built not on GEOS (which is a C library). And I think we do support collections since the test_match_geopandas_series test passed naturally (self.geoms in that test contains collections). Could you add a single test in test_geoseries.py that tests a GEOMETRYCOLLECTION with a non-zero z-arg?

e.g. GeometryCollection(Point(1, 1), LineString([(1, 1), (0, 1), (1, 0)])) and force_3d(3)

petern48 · 2025-11-20T06:32:19Z

python/tests/geopandas/test_match_geopandas_series.py

+            sgpd_result = GeoSeries(geom).force_3d()
+            gpd_result = gpd.GeoSeries(geom).force_3d()


Suggested change

sgpd_result = GeoSeries(geom).force_3d()

gpd_result = gpd.GeoSeries(geom).force_3d()

sgpd_result = GeoSeries(geom).force_3d(4)

gpd_result = gpd.GeoSeries(geom).force_3d(4)

This is a minor change, and I don't think Sedona would fail it, but I think this would be a slightly better test if we test a non-zero z argument. These "match" tests in test_match_geopandas_series.py are the most comprehensive/powerful, so I'd prefer to avoid a common default case like 0. e.g. the chances that we have a bug that always set z=0 in some case is much higher than the chance that it always set z=4.

chay0112 · 2025-11-20T18:16:18Z

Looks good. Just some minor feedback about improving the docs and the tests. Should be good to merge afterwards.

@petern48 Thanks Peter, Great insights as always. Just addressed them.

petern48 · 2025-11-21T02:39:51Z

python/tests/geopandas/test_geoseries.py

+                LineString([(1, 1, 2), (0, 1, 2), (1, 0, 2)]),
+                Polygon([(0, 0, 3), (0, 10, 3), (10, 10, 3), (0, 0, 3)]),


Suggested change

LineString([(1, 1, 2), (0, 1, 2), (1, 0, 2)]),

Polygon([(0, 0, 3), (0, 10, 3), (10, 10, 3), (0, 0, 3)]),

LineString([(1, 1, 3), (0, 1, 3), (1, 0, 3)]),

Polygon([(0, 0, 4), (0, 10, 4), (10, 10, 4), (0, 0, 4)]),

I noticed our tests here were slightly buggy. Here's what the expected results should look like. These geoms correspond to the 3 and 4 in z = [0, 2, 2, 3, 4, 5]. What's concerning is that the tests pass anyway. Experimenting locally, I noticed we can change the z-coordinate however we like, and the tests still pass. I think what's happening is that the check_sgpd_equals_gpd() doesn't yet check anything past the x and y dimensions...

(sigh) Unfortunately, what this means is that we'll need to modify the test code to test this properly before we can merge this PR. I filed an issue for this: #2513. You're welcome to try taking a stab at it or just move on to something else (while keeping this PR open). My time is limited, so I wouldn't be able to investigate it for at least a few days. I don't love having to put a hold on merging your great work here, but this is just how it is sometimes. Sorry about this :(

Got it, Thanks for the insights Peter. No problem, I'll move on to a new one.

petern48 · 2025-11-23T15:38:03Z

Alright @chay0112, the fix has been merged. Think this should be ready once that last suggestion is accepted.

chay0112 · 2025-11-24T06:05:54Z

Alright @chay0112, the fix has been merged. Think this should be ready once that last suggestion is accepted.

Thanks for your fix Peter, I believe I need to rephrase my tests in test_match_geopandas_series.py because things like Line Strings to Linear Rings and Polygons to Multipolygons would fail

chay0112 · 2025-11-24T19:36:22Z

@petern48 I made some adjustments to the tests in test_match_geopandas_series.py because the previous assertions were failing for M and ZM geometry cases. Please review the changes and let me know if there’s a better or cleaner approach. Feedback is welcome.

petern48 · 2025-11-25T02:53:25Z

Responding to the deletions in this commit: f90875a. Here are the failures from the test_match_geopandas tests (I ran locally). Next time, ideally report back when you see behavior like this, instead of relying on me to investigate myself (sometimes, it may take longer that way). Sometimes, it's reasonable to skip; sometimes it's major enough to just put a halt to it. Alright let's see.

Error for LinearRing:

ValueError: Geometry equality check failed for LINESTRING Z (0 0 4, 1 0 4, 1 1 4, 0 1 4, 0 0 4) and LINEARRING Z (0 0 4, 1 0 4, 1 1 4, 0 1 4, 0 0 4)

Sedona is returning the equivalent of a LineString. Sedona doesn't support LinearRings since LineStrings are enough. Yep this is fine to skip ✅.

Error for MultiPolygon:

ValueError: Geometry equality check failed for POLYGON Z ((0 0 4, 0 1 4, 1 0 4, 0 0 4), (0.1 0.1 4, 0.1 0.2 4, 0.2 0.1 4, 0.1 0.1 4)) and MULTIPOLYGON Z (((0 0 4, 0 1 4, 1 0 4, 0 0 4), (0.1 0.1 4, 0.1 0.2 4, 0.2 0.1 4, 0.1 0.1 4))

Ah interesting, it's the same geometry, but we return polygon instead of multipolygon. This is a minor bug in Sedona we found. Ideally, we want to return MultiPolygon. I dug in real quick and submitted a fix for this: #2526

Trying it on PostGIS, I see returning MultiPolygon is the desired behavior as well

SELECT ST_AsText(ST_Force3D(ST_GeomFromText('MULTIPOLYGON(((0 0, 0 1, 1 0, 0 0), (0.1 0.1, 0.1 0.2, 0.2 0.1, 0.1 0.1)))'), 4));
-- MULTIPOLYGON Z (((0 0 4,0 1 4,1 0 4,0 0 4),(0.1 0.1 4,0.1 0.2 4,0.2 0.1 4,0.1 0.1 4)))

Error for GeometryCollection

ValueError: Geometry equality check failed for GEOMETRYCOLLECTION Z (MULTIPOINT Z ((0 0 4), (1 1 4)), MULTILINESTRING Z ((0 0 4, 1 1 4), (2 2 4, 3 3 4)), POLYGON Z ((0 0 4, 0 1 4, 1 0 4, 0 0 4), (0.1 0.1 4, 0.1 0.2 4, 0.2 0.1 4, 0.1 0.1 4))) and GEOMETRYCOLLECTION Z (MULTIPOINT Z ((0 0 4), (1 1 4)), MULTILINESTRING Z ((0 0 4, 1 1 4), (2 2 4, 3 3 4)), MULTIPOLYGON Z (((0 0 4, 0 1 4, 1 0 4, 0 0 4), (0.1 0.1 4, 0.1 0.2 4, 0.2 0.1 4, 0.1 0.1 4))))

Fails for the same reason as multipolygon (since it has a multipolygon inside of it).

Error for M coordinates. Hmm, yeah this one is interesting. Give me a bit more time to figure out what to do here.

# For POINT M (1 2 3) input
ValueError: Geometry equality check failed for POINT Z (1 2 7.5) and POINT M (1 2 3)

# For POINT ZM (1 2 3 4) input
ValueError: Geometry equality check failed for POINT Z (1 2 3) and POINT Z (1 2 7.5)

chay0112 · 2025-11-25T04:34:49Z

Responding to the deletions in this commit: f90875a. Here are the failures from the test_match_geopandas tests (I ran locally). Next time, ideally report back when you see behavior like this, instead of relying on me to investigate myself (sometimes, it may take longer that way). Sometimes, it's reasonable to skip; sometimes it's major enough to just put a halt to it. Alright let's see.

Error for LinearRing:
ValueError: Geometry equality check failed for LINESTRING Z (0 0 4, 1 0 4, 1 1 4, 0 1 4, 0 0 4) and LINEARRING Z (0 0 4, 1 0 4, 1 1 4, 0 1 4, 0 0 4)
Sedona is returning the equivalent of a LineString. Sedona doesn't support LinearRings since LineStrings are enough. Yep this is fine to skip ✅.

Error for MultiPolygon:
ValueError: Geometry equality check failed for POLYGON Z ((0 0 4, 0 1 4, 1 0 4, 0 0 4), (0.1 0.1 4, 0.1 0.2 4, 0.2 0.1 4, 0.1 0.1 4)) and MULTIPOLYGON Z (((0 0 4, 0 1 4, 1 0 4, 0 0 4), (0.1 0.1 4, 0.1 0.2 4, 0.2 0.1 4, 0.1 0.1 4))
Ah interesting, it's the same geometry, but we return polygon instead of multipolygon. This is a minor bug in Sedona we found. Ideally, we want to return MultiPolygon. I dug in real quick and submitted a fix for this: #2526

Trying it on PostGIS, I see returning MultiPolygon is the desired behavior as well
SELECT ST_AsText(ST_Force3D(ST_GeomFromText('MULTIPOLYGON(((0 0, 0 1, 1 0, 0 0), (0.1 0.1, 0.1 0.2, 0.2 0.1, 0.1 0.1)))'), 4));
-- MULTIPOLYGON Z (((0 0 4,0 1 4,1 0 4,0 0 4),(0.1 0.1 4,0.1 0.2 4,0.2 0.1 4,0.1 0.1 4)))
Error for GeometryCollection
ValueError: Geometry equality check failed for GEOMETRYCOLLECTION Z (MULTIPOINT Z ((0 0 4), (1 1 4)), MULTILINESTRING Z ((0 0 4, 1 1 4), (2 2 4, 3 3 4)), POLYGON Z ((0 0 4, 0 1 4, 1 0 4, 0 0 4), (0.1 0.1 4, 0.1 0.2 4, 0.2 0.1 4, 0.1 0.1 4))) and GEOMETRYCOLLECTION Z (MULTIPOINT Z ((0 0 4), (1 1 4)), MULTILINESTRING Z ((0 0 4, 1 1 4), (2 2 4, 3 3 4)), MULTIPOLYGON Z (((0 0 4, 0 1 4, 1 0 4, 0 0 4), (0.1 0.1 4, 0.1 0.2 4, 0.2 0.1 4, 0.1 0.1 4))))
Fails for the same reason as multipolygon (since it has a multipolygon inside of it).

Error for M coordinates. Hmm, yeah this one is interesting. Give me a bit more time to figure out what to do here.
# For POINT M (1 2 3) input
ValueError: Geometry equality check failed for POINT Z (1 2 7.5) and POINT M (1 2 3)

# For POINT ZM (1 2 3 4) input
ValueError: Geometry equality check failed for POINT Z (1 2 3) and POINT Z (1 2 7.5)

@petern48 Sure, I'll report the anomalies from next time, Thanks for a very clear explanation. Take your time. Thanks

petern48

Sorry for the delay @chay0112. Thanks for being patient. I took another look and saw that Sedona follows PostGIS behavior (which is what we want), and Geopandas behavior apparently is just different. I think we can keep this difference in behavior, but specify this difference in the docstring. Left some suggestions, but I think this should be ready once addressed.

petern48 · 2025-11-28T22:52:16Z

python/tests/geopandas/test_match_geopandas_series.py

+            pytest.skip("geopandas force_3d requires version 1.0.0 or higher")
+        # 1) Promote 2D to 3D with z = 4
+        for geom in self.geoms:
+            if isinstance(geom[0], (LinearRing, GeometryCollection, MultiPolygon)):


Suggested change

if isinstance(geom[0], (LinearRing, GeometryCollection, MultiPolygon)):

if isinstance(geom[0], (LinearRing)):

This should theoretically pass now.

Hi Peter, Thanks for your suggestions.
I believe if we remove Multipolygon and GeometryCollections the test would still fail for geoms, something like below
`FAILED
============================================================================== FAILURES ==============================================================================
_______________________________________________________________ TestMatchGeopandasSeries.test_force_3d _______________________________________________________________

python/tests/geopandas/test_match_geopandas_series.py:890:

python/tests/geopandas/test_geopandas_base.py:70: in check_sgpd_equals_gpd
cls.assert_geometry_almost_equal(a, e, tolerance)

cls = <class 'tests.geopandas.test_match_geopandas_series.TestMatchGeopandasSeries'>
left_geom = <POLYGON Z ((0 0 4, 0 1 4, 1 0 4, 0 0 4), (0.1 0.1 4, 0.1 0.2 4, 0.2 0.1 4, ...>
right_geom = <MULTIPOLYGON Z (((0 0 4, 0 1 4, 1 0 4, 0 0 4), (0.1 0.1 4, 0.1 0.2 4, 0.2 0...>, tolerance = 0.01

ValueError: Geometry equality check failed for POLYGON Z ((0 0 4, 0 1 4, 1 0 4, 0 0 4), (0.1 0.1 4, 0.1 0.2 4, 0.2 0.1 4, 0.1 0.1 4)) and MULTIPOLYGON Z (((0 0 4, 0 1 4, 1 0 4, 0 0 4), (0.1 0.1 4, 0.1 0.2 4, 0.2 0.1 4, 0.1 0.1 4)))
`

Ah, I realize you're not yet familiar with iterating with Java changes. You need to update your code to reflect the changes I made in this PR. I'm guessing you likely followed this guide for dev env setup, since it was mentioned in the original geopandas EPIC issue.

First, make sure you pull in the latest main and merge it into this branch.

Now, even after you get the changes, you need to rebuild the Sedona JAR and move it to your SPARK_HOME directory. These are the two commands in section 3) of those compile docs.

Basically, you just need to redo the 2 commands in section 3, and then the changes should be reflected when you run pytest.

mvn clean install -DskipTests -Dgeotools cp spark-shaded/target/sedona-spark-shaded-*.jar $SPARK_HOME/jars/

Note, if you run into trouble with the first command succeeding, you may need to specify versions in the mvn build command. e.g. Here's what I run each time: mvn clean install -DskipTests -Dgeotools -Dspark=3.4 -Pscala2.12. The same command likely would still work for you if you set up the same versions as in those docs. @chay0112 Give it a try. Knowing how to do this is also very helpful for getting into fixing small Java bugs, like I had done earlier.

Cool, Thanks for letting me know.

petern48 · 2025-11-28T23:08:40Z

python/sedona/spark/geopandas/base.py

+        """Force the dimensionality of a geometry to 3D.
+
+        2D geometries will get the provided Z coordinate; 3D geometries
+        are unchanged (unless their Z coordinate is ``np.nan``).


Suggested change

are unchanged (unless their Z coordinate is ``np.nan``).

are unchanged (unless their Z coordinate is ``np.nan``).

Note: Sedona's behavior may differ from Geopandas' for M and ZM geometries.

For M geometries, Sedona will replace the M coordinate and add the Z coordinate.

For ZM geometries, Sedona will drop the M coordinate and retain the Z coordinate.

petern48 · 2025-12-02T06:26:54Z

python/tests/geopandas/test_geoseries.py

+            ]
+        )
+        result = s.force_3d(z)
+        self.check_sgpd_equals_gpd(result, expected)


Could you add these cases below?

ST_Force3D(st_geomfromtext('POINT M (1 2 3)'), 7.5) -- Result: POINT Z(1 2 7.5) ST_Force3D(st_geomfromtext('POINT ZM (1 2 3 4)'), 7.5) -- Result: POINT Z(1 2 3)

For those curious about the difference in behavior, Geopandas returns the following instead. It's strange and inconsistent imo... I would guess it's unintentional and just a result of adding M support later on.

gpd.GeoSeries([shapely.wkt.loads("POINT M (1 2 3)")]).force_3d(7.5) 0 POINT M (1 2 3) gpd.GeoSeries([shapely.wkt.loads("POINT ZM (1 2 3 4)")]).force_3d(7.5) 0 POINT Z (1 2 7.5)

petern48 · 2025-12-02T06:38:24Z

Also, could you mention "geopandas" in the PR title (somehow). The PR title eventually becomes the commit message when merged, so it's helpful to specify these details. Could be something like one of the following (I typically do the first one).

[GH-XXXX] Geopandas: Implement force_3d
[GH-XXXX] Implement geopandas force_3d

chay0112 · 2025-12-03T20:15:39Z

@petern48 Just addressed all the comments. It looks like the docs build is failing, and I noticed the same issue in other PRs as well. I'm not sure of the root cause, but if that's not a blocker, this PR is ready for review. No rush though !

petern48 · 2025-12-04T05:46:31Z

Yeah, the doc build failure should be unrelated. Recently was fixed here: #2551

petern48

Thanks for sticking it through!

chay0112 requested a review from jiayuasu as a code owner November 20, 2025 04:09

github-actions bot added the sedona-python label Nov 20, 2025

petern48 reviewed Nov 20, 2025

View reviewed changes

petern48 reviewed Nov 21, 2025

View reviewed changes

chay0112 force-pushed the feat/force-3d-impl branch from 9fafeb5 to f90875a Compare November 24, 2025 18:57

petern48 mentioned this pull request Nov 25, 2025

bug: ST_Force3d() returns polygon when input is multipolygon with a single nested polygon #2525

Closed

petern48 reviewed Dec 2, 2025

View reviewed changes

chay0112 changed the title ~~[GH-2504] Implement force_3d~~ [GH-2504] Geopandas: Implement force_3d Dec 2, 2025

chay0112 added 5 commits December 3, 2025 11:27

initial draft for force 3d

e9fe11a

refactored test cases

c0ee218

addressed testcases and few nits

9d62bad

modified tests according to changes in source code

819a3ee

Refactored the structure of tests

52f268b

chay0112 force-pushed the feat/force-3d-impl branch from f90875a to 52f268b Compare December 3, 2025 18:29

petern48 approved these changes Dec 4, 2025

View reviewed changes

jiayuasu merged commit c7e42cb into apache:master Dec 4, 2025
30 of 31 checks passed

		>>> from shapely import Polygon, LineString, Point
		>>> s = geopandas.GeoSeries(

		2D geometries will get the provided Z coordinate; 3D geometries
		are unchanged (unless their Z coordinate is ``np.nan``).

		Note that for empty geometries, 3D is only supported since GEOS 3.9 and then
		still only for simple geometries (non-collections).

		sgpd_result = GeoSeries(geom).force_3d()
		gpd_result = gpd.GeoSeries(geom).force_3d()

		LineString([(1, 1, 2), (0, 1, 2), (1, 0, 2)]),
		Polygon([(0, 0, 3), (0, 10, 3), (10, 10, 3), (0, 0, 3)]),

	if isinstance(geom[0], (LinearRing, GeometryCollection, MultiPolygon)):
	if isinstance(geom[0], (LinearRing)):

-        are unchanged (unless their Z coordinate is ``np.nan``).
+        are unchanged (unless their Z coordinate is ``np.nan``).
+        Note: Sedona's behavior may differ from Geopandas' for M and ZM geometries.
+        For M geometries, Sedona will replace the M coordinate and add the Z coordinate.
+        For ZM geometries, Sedona will drop the M coordinate and retain the Z coordinate.

[GH-2504] Geopandas: Implement force_3d #2512

[GH-2504] Geopandas: Implement force_3d #2512

Uh oh!

Conversation

chay0112 commented Nov 20, 2025

Did you read the Contributor Guide?

Is this PR related to a ticket?

What changes were proposed in this PR?

How was this patch tested?

Did this PR include necessary documentation updates?

Uh oh!

petern48 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chay0112 commented Nov 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

petern48 commented Nov 23, 2025

Uh oh!

chay0112 commented Nov 24, 2025

Uh oh!

chay0112 commented Nov 24, 2025

Uh oh!

petern48 commented Nov 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chay0112 commented Nov 25, 2025

Uh oh!

petern48 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chay0112 Dec 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

petern48 commented Dec 2, 2025

Uh oh!

chay0112 commented Dec 3, 2025

Uh oh!

petern48 commented Dec 4, 2025

Uh oh!

petern48 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

chay0112 commented Nov 20, 2025 •

edited

Loading

petern48 commented Nov 25, 2025 •

edited

Loading

chay0112 Dec 2, 2025 •

edited

Loading