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

MNT: recommend write_crs & deprecate set_crs #793

Merged
merged 9 commits into from
Jul 8, 2024

Conversation

dluks
Copy link
Contributor

@dluks dluks commented Jul 3, 2024

There is some confusion around when to use write_crs versus set_crs. This adds a small note to generally prefer write_crs when CRS persistence is needed. Fixes #743

One remaining nit that I would like to clarify in the documentation as well: when exactly should people use rio.set_crs() and not rio.write_crs()?

There is some confusion around when to use write_crs versus set_crs. This adds a small note to generally prefer write_crs when CRS persistence is needed. Fixes corteva#743
@snowman2
Copy link
Member

snowman2 commented Jul 3, 2024

Thanks! Would you also be willing to add this to the set_crs docstrings?

@snowman2
Copy link
Member

snowman2 commented Jul 3, 2024

when exactly should people use rio.set_crs() and not rio.write_crs()?

I would say never use rio.set_crs due to its instability. It was added a while a go and probably should be deprecated.

@dluks
Copy link
Contributor Author

dluks commented Jul 3, 2024

@snowman2 I've pushed two commits that address the set_crs definition. The first adds a warning to the docstring and the second emits an actual DeprecationWarning when using set_crs. Not sure if raising an actual warning is what you had in mind, so if not I can revert it.

rioxarray/rioxarray.py Outdated Show resolved Hide resolved
rioxarray/rioxarray.py Outdated Show resolved Hide resolved
rioxarray/rioxarray.py Outdated Show resolved Hide resolved
@snowman2 snowman2 added this to the 0.16.0 milestone Jul 8, 2024
@snowman2 snowman2 added bug Something isn't working documentation Documentation related issue labels Jul 8, 2024
Co-authored-by: Alan D. Snow <alansnow21@gmail.com>
@snowman2 snowman2 changed the title docs: add clarification regarding write_crs and set_crs MNT: recommend write_crs & deprecate set_crs Jul 8, 2024
@snowman2
Copy link
Member

snowman2 commented Jul 8, 2024

Thanks @dluks 👍

@snowman2 snowman2 merged commit b877e41 into corteva:master Jul 8, 2024
17 of 19 checks passed
weiji14 added a commit to GenericMappingTools/pygmt that referenced this pull request Jul 9, 2024
Rioxarray 0.16.0 has deprecated the use of `set_crs` in favour of `write_crs`. Xref corteva/rioxarray#793
weiji14 added a commit to GenericMappingTools/pygmt that referenced this pull request Jul 9, 2024
* Replace rio.set_crs with rio.write_crs in load_tile_map function

Rioxarray 0.16.0 has deprecated the use of `set_crs` in favour of `write_crs`. Xref corteva/rioxarray#793

* Add spatial_ref coordinate to load_tile_map's doctest output

The `write_crs` command will write an extra grid_mapping attribute to the encoding that shows up in the coordinates.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Documentation related issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Document difference between set_crs and write_crs
2 participants