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

Should .reset_encoding be .drop_encoding? #8259

Closed
max-sixty opened this issue Sep 30, 2023 · 1 comment · Fixed by #8287
Closed

Should .reset_encoding be .drop_encoding? #8259

max-sixty opened this issue Sep 30, 2023 · 1 comment · Fixed by #8287

Comments

@max-sixty
Copy link
Collaborator

What is your issue?

Not the greatest issue facing the universe — but for the cause of consistency — should .reset_encoding be .drop_encoding, since it drops all encoding attributes?

For comparison:

  • .reset_coords — "Given names of coordinates, reset them to become variables."
  • '.drop_vars` — "Drop variables from this dataset."

Also ref #8258

@jhamman
Copy link
Member

jhamman commented Oct 7, 2023

🤚 .reset_encoding was indeed by addition. In hindsight, drop_encoding would have been a better name. Very open to seeing this changed to align with the other drop_foo methods.

max-sixty added a commit to max-sixty/xarray that referenced this issue Oct 8, 2023
max-sixty added a commit that referenced this issue Oct 12, 2023
* Rename `reset_encoding` to `drop_encoding`

Closes #8259

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Update dataarray.py

Co-authored-by: Illviljan <[email protected]>

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Update xarray/core/dataarray.py

Co-authored-by: Michael Niklas  <[email protected]>

* Update xarray/core/variable.py

Co-authored-by: Michael Niklas  <[email protected]>

* Update xarray/core/dataset.py

Co-authored-by: Michael Niklas  <[email protected]>

* api

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Illviljan <[email protected]>
Co-authored-by: Michael Niklas <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants