Skip to content

Conversation

@TomNicholas
Copy link
Member

Does this need a test?

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 @TomNicholas

TomNicholas and others added 3 commits April 29, 2021 13:27
Comment on lines +473 to +480
# 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,
)
Copy link
Collaborator

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?

Copy link
Member Author

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.

Copy link
Collaborator

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

Copy link
Member Author

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.

@alexamici
Copy link
Collaborator

@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 lock argument.

@keewis
Copy link
Collaborator

keewis commented May 4, 2021

I think this should have been closed by #5256. Please reopen if I misunderstood.

@keewis keewis closed this May 4, 2021
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.

lock kwarg needs a deprecation cycle?

4 participants