Skip to content

[GH-3017] Implement rotate#3018

Open
oglego wants to merge 1 commit into
apache:masterfrom
oglego:feature/implement-rotate
Open

[GH-3017] Implement rotate#3018
oglego wants to merge 1 commit into
apache:masterfrom
oglego:feature/implement-rotate

Conversation

@oglego
Copy link
Copy Markdown

@oglego oglego commented May 31, 2026

Did you read the Contributor Guide?

Is this PR related to a ticket?

What changes were proposed in this PR?

Implement rotate

How was this patch tested?

Added tests, verified success of all tests with:

pytest -v tests/geopandas/test_geoseries.py

117 passed, 44108 warnings in 140.69s (0:02:20)

Verified formatting with:

make check

Did this PR include necessary documentation updates?

  • Yes, I have updated the documentation.

@oglego oglego requested a review from jiayuasu as a code owner May 31, 2026 17:37
@oglego
Copy link
Copy Markdown
Author

oglego commented May 31, 2026

Hey @petern48,

I have tried to follow along with the instructions on ticket 2230 and PR example 2231 but if I missed anything on this please let me know! Thank you for putting those instructions together they were very helpful!

Copy link
Copy Markdown
Member

@petern48 petern48 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this. I've done an initial scan and left some feedback

Examples
--------
>>> from shapely.geometry import Point, LineString, Polygon
>>> s = geopandas.GeoSeries(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
>>> s = geopandas.GeoSeries(
>>> from sedona.spark.geopandas import GeoSeries
>>> s = GeoSeries(

we should update the import for sedona instead of geopandas

Comment on lines +966 to +967
sgpd_result = GeoSeries(geom).rotate(45, origin=(0, 0))
gpd_result = gpd.GeoSeries(geom).rotate(45, origin=(0, 0))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we update these to an origin other than (0, 0), just to make sure it works? maybe something like (1, 2).

can we also add add a case for testing "centroid" and a Point() object as input for origin field to make sure they work?

gpd_s = gpd.GeoSeries(geoms)

# Test default (degrees, origin='center')
self.check_sgpd_equals_gpd(s.rotate(90), gpd_s.rotate(90))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test_geoseries.py should be testing the hard-coded expected outputs instead of comparing it directly to the result of what geopandas returns (that's what test_match_geopandas_series.py` is for. Could you update all of these examples to compare to the hard-coded expected output geometries?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Geopandas: Implement rotate

2 participants