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

backwards incompatible changes in reductions #6408

Closed
dcherian opened this issue Mar 24, 2022 · 2 comments · Fixed by #6409
Closed

backwards incompatible changes in reductions #6408

dcherian opened this issue Mar 24, 2022 · 2 comments · Fixed by #6409

Comments

@dcherian
Copy link
Contributor

What is your issue?

I merged #5950 but forgot that it included some backward-incompatible changes (Sorry! this came up in #6403 thanks to @mathause for spotting.)

  1. Arguments like keep_attrs, axis are now keyword-only.
  2. Some reductions had the 3rd position arg as keep_attrs and in other cases it was axis.

These have been standardized now, and only dim is accepted without kwarg-name.

@pydata/xarray Should we add a deprecation cycle (#5531)? Point (2) above will make it a little messy.

At the very least we should add a deprecation notice before releasing.

@dcherian dcherian added needs triage Issue that has not been reviewed by xarray team member and removed needs triage Issue that has not been reviewed by xarray team member labels Mar 24, 2022
@shoyer
Copy link
Member

shoyer commented Mar 24, 2022

I think this is probably fine without a deprecation cycle. This is a very easy fix for users.

@max-sixty
Copy link
Collaborator

@shoyer do you think we need a deprecation cycle for those in #6403? It would be less work if we didn't.

I was thinking that for some of them we did need a depr. cycle, but only because I've seen many cases where specific methods don't use kwargs — e.g. .diff(dim, 2). WDYT?

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 a pull request may close this issue.

3 participants