-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add deprecation warnings for lock kwarg #5237
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
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.
Thanks @TomNicholas
Co-authored-by: Maximilian Roos <[email protected]>
Co-authored-by: Maximilian Roos <[email protected]>
# TODO remove after v0.19 | ||
if kwargs.pop("lock", None): | ||
warnings.warn( | ||
"The kwarg 'lock' has been deprecated, and is now " | ||
"ignored. In the future passing lock will " | ||
"raise an error.", | ||
DeprecationWarning, | ||
) |
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.
should we include the target version in the warning and remove the comment?
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.
I did that originally but Max said above that it might be better to leave it out... I think this is fine to be honest.
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.
hmm... in any case, I think it would be good to keep the error message for open_dataset
and open_dataarray
synchronized
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.
We could always add a specific date of deprecation later on, even if that would extend the cycle. It doesn't really matter if this check just sits here for a while I suppose.
Oh I didn't mean to make them different! Will fix now.
@TomNicholas see my comment here: #5073 (comment) I think the best course for now is to only raise the deprecation warning in zarr and pydap, as al other in-tree backends actually support the |
I think this should have been closed by #5256. Please reopen if I misunderstood. |
Does this need a test?
lock
kwarg needs a deprecation cycle? #5073pre-commit run --all-files
whats-new.rst