Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[proposal] Add CRS class similar to the CRS in rasterio #714

Closed
snowman2 opened this issue Jan 24, 2019 · 8 comments
Closed

[proposal] Add CRS class similar to the CRS in rasterio #714

snowman2 opened this issue Jan 24, 2019 · 8 comments
Assignees
Milestone

Comments

@snowman2
Copy link
Contributor

Are there plans to update the CRS functionality in fiona to match the current version in rasterio?

@sgillies sgillies added this to the 1.8.5 milestone Jan 24, 2019
@sgillies
Copy link
Member

@snowman2 I consider it a must do for the next release, but haven't begun work on it.

@snowman2
Copy link
Contributor Author

Good to hear 👍

@sgillies
Copy link
Member

sgillies commented Jan 26, 2019

@snowman2 one thing to keep in mind: we'll need to coordinate with @jorisvandenbossche and the geopandas team to make sure that we don't cause trouble like we did for xarray: pydata/xarray#2715. I would have advised the xarray project not to bind itself to the microformatting of CRS.to_string, we should do a check that geopandas isn't doing the same thing.

Also: I think we'll need to expand our tests to make sure that CRS.from_epsg(n) == {'init': 'epsg:{}'.format(n)} remains true for all n, as I see this is a recurring pattern in geopandas.

@snowman2
Copy link
Contributor Author

That sounds like a good plan to me. Maybe a Dev release would be a good idea to allow the geopandas team to adjust and give feedback before the release?

@jorisvandenbossche
Copy link
Member

Currently, we rely on fiona's CRS in one place in geopandas.read_file, where we assume that the crs attribute of a Collection is a proj4-like dictionary.

I assume the idea would be that this would become a actual CRS class? Although this seems less backwards compatible than the change in rasterio where there was already such a class before as well (as far as I know fiona has not yet such a class but just works with a proj4 dict, at least as user interface?)

In any case, as long as we are aware of it in advance, we can easily add code in GeoPandas to do the correct thing depending on the Fiona version.

@sgillies
Copy link
Member

sgillies commented Feb 8, 2019

@jorisvandenbossche @snowman2 I just submitted geopandas/geopandas#913 to give geopandas more compatibility. Yes? No?

@snowman2
Copy link
Contributor Author

@sgillies, I know you have many things you are working on, so if it would be useful for me to work on an implementation (or just a part of it), just let me know and I would be happy to help out.

@sgillies sgillies modified the milestones: 1.8.5, 1.9 Mar 14, 2019
@sgillies sgillies self-assigned this Mar 14, 2019
jorisvandenbossche pushed a commit to geopandas/geopandas that referenced this issue Apr 18, 2019
@sgillies sgillies modified the milestones: 1.9, CRS class May 18, 2022
sgillies added a commit that referenced this issue May 31, 2022
sgillies added a commit that referenced this issue Jun 1, 2022
* Adopt rasterio's CRS class

Resolves #714

* Remove references to rasterio, note version added (1.9.0)

* Mark tests that require GDAL >= 3.3.0
@snowman2
Copy link
Contributor Author

Thanks @sgillies 👍

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

No branches or pull requests

3 participants