-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Raise on inplace=True #3260
Raise on inplace=True #3260
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would also be nice to also update these methods to avoid using the inplace argument internally (e.g., to replace it with a literal Boolean if necessary).
xarray/core/utils.py
Outdated
"removed in a future version of xarray.", | ||
FutureWarning, | ||
stacklevel=3, | ||
raise ValueError( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably be TypeError, which is what Python uses for unrecognized arguments.
Great - what are your thoughts on |
All done, unless anyone has thoughts re |
Could we make |
Yes, good idea. Currently |
Yes, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @max-sixty !
* upstream/master: Initialize empty or full DataArray (pydata#3159) Raise on inplace=True (pydata#3260) added support for rasterio geotiff tags (pydata#3249) Remove sel_points (pydata#3261) Fix sparse ops that were calling bottleneck (pydata#3254) New feature of filter_by_attrs added (pydata#3259) Update filter_by_attrs to use 'variables' instead of 'data_vars' (pydata#3247)
black . && mypy . && flake8
whats-new.rst
for all changes andapi.rst
for new APIDo we want to raise an explicit error for another minor release (rather than remove all traces of
inplace
)? I'd vote +0.5 - it's likely that some people don't check deprecation warnings and this states the problem clearlyWhat do we want to do re
.update
(#2951)?