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

rm inplace arg to rename #4561

Merged
merged 3 commits into from
Nov 7, 2020
Merged

rm inplace arg to rename #4561

merged 3 commits into from
Nov 7, 2020

Conversation

raybellwaves
Copy link
Contributor

@raybellwaves raybellwaves commented Nov 3, 2020

This PR follows a TypeError I got

ds = xr.open_dataset('https://thredds.ucar.edu/thredds/dodsC/grib/NCEP/GFS/Global_0p25deg/Best')
ds.rename({'lat': 'latitude', 'lon': 'longitude'}, inplace=True)
TypeError: The `inplace` argument has been removed from xarray. You can achieve an identical effect with python's standard assignment.

I saw the option of the inplace in the docs
http://xarray.pydata.org/en/stable/generated/xarray.Dataset.rename.html

Removing this argument.

TODO:

  • rm test_rename_inplace

@pep8speaks
Copy link

pep8speaks commented Nov 3, 2020

Hello @raybellwaves! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-11-04 21:27:35 UTC

@raybellwaves raybellwaves marked this pull request as draft November 3, 2020 12:11
Copy link
Collaborator

@max-sixty max-sixty left a comment

Choose a reason for hiding this comment

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

Thanks @raybellwaves !

@max-sixty
Copy link
Collaborator

The reason this was here is so people get a useful error as they make the transition.

It's probably been there long enough that we can remove it (no strong view though)

@max-sixty
Copy link
Collaborator

One alternative is to remove inplace from the signature, and instead extract it from kwargs. As above, I would be fine removing it, though.

@mathause
Copy link
Collaborator

mathause commented Nov 4, 2020

This was in 0.13 http://xarray.pydata.org/en/stable/whats-new.html#v0-13-0-17-sep-2019 I actually don't know if we have an official policy for deprecations? Two major versions?

Should all of these checks be removed? #3260

@max-sixty
Copy link
Collaborator

I don't think we have a strict policy but a few releases seem fine. Eventually they should go all go; but fine to merge them as we go too.

@max-sixty
Copy link
Collaborator

Great, merging! Thanks @raybellwaves !

@max-sixty
Copy link
Collaborator

Erm, actually I think you may have to take it off WIP first

@raybellwaves raybellwaves marked this pull request as ready for review November 5, 2020 14:39
@max-sixty max-sixty merged commit c02b805 into pydata:master Nov 7, 2020
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.

4 participants