Skip to content

Comments

Eliminate much to_epsg() usage and speed up __eq__()#3216

Merged
sgillies merged 4 commits intomaint-1.4from
issue3207
Oct 22, 2024
Merged

Eliminate much to_epsg() usage and speed up __eq__()#3216
sgillies merged 4 commits intomaint-1.4from
issue3207

Conversation

@sgillies
Copy link
Member

@sgillies sgillies commented Oct 20, 2024

Change to CRS.__eq__() is based on #3208.

Several evaluations of to_epsg(), which is slow, are replaced by a check of CRS._epsg.

A notable change in behavior is that CRS.from_epsg(4326).to_dict() no longer returns {"init": "epsg:4326"}, but the full PROJ definition. PROJ4 init files aren't quite deprecated yet, but we need to steer users away from this. See #1809 (comment) (which I'll try to replace with a better reference).

Additionally: stop asserting on "init=epsg:dddd" as this is no
longer recommended usage.
@sgillies sgillies self-assigned this Oct 20, 2024
Copy link
Member

@vincentsarago vincentsarago left a comment

Choose a reason for hiding this comment

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

🚀

@pytest.mark.parametrize("crs", [CRS.from_epsg(4326), CRS.from_string("EPSG:4326")])
def test_epsg_4326_ogc_crs84(crs):
"""EPSG:4326 not equivalent to OGC:CRS84."""
assert CRS.from_string("OGC:CRS84") != crs
Copy link
Member Author

@sgillies sgillies Oct 21, 2024

Choose a reason for hiding this comment

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

Note: while GDAL and Rasterio may treat these as equivalent, they are not equivalent.

I don't think it matters very much. Nobody uses "OGC:CRS84" except me 😆 The rest of the FOSS4G world still uses "EPSG:4326" and won't be making this comparison. But it's still good to be correct, I think.

Copy link
Member

Choose a reason for hiding this comment

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

🚀

@vincentsarago vincentsarago self-requested a review October 21, 2024 16:22
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.

3 participants