Skip to content

change default in ds.chunk and datarray.chunk variable.chunk #4633

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

Merged
merged 3 commits into from
Dec 10, 2020
Merged

change default in ds.chunk and datarray.chunk variable.chunk #4633

merged 3 commits into from
Dec 10, 2020

Conversation

aurghs
Copy link
Collaborator

@aurghs aurghs commented Dec 1, 2020

The aim is to split the PR #4595 in small PRs.
The scope of this smaller PR is to modify the default of chunks in dataset.chunk to align the behaviour to xr.open_dataset.
The main changes are:

  • Modify the default of chunk in dataset.chunk, datarray.chunk and variable.chunk from None to {}.
  • If the user pass chunk=None inside is set to {}
  • Add a future warning to advice that the usage of None will raise an error in the future.

Note that the changes currently don't modify the behaviour of dataset.chunk

@alexamici alexamici self-requested a review December 1, 2020 17:21
Copy link
Collaborator

@alexamici alexamici left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the agreed change to the .chunks default value.

There is no user change now, as None is still accepted, but deprecated to avoid that the meaning is different from chunks=None in xr.open_dataset.

Copy link
Member

@shoyer shoyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @jhamman for a second look -- we talked about this a few weeks ago

@pep8speaks
Copy link

pep8speaks commented Dec 9, 2020

Hello @aurghs! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-12-10 07:59:12 UTC

if bad_dims:
raise ValueError(
"some chunks keys are not dimensions on this " "object: %s" % bad_dims
)

variables = {
k: _maybe_chunk(k, v, chunks, token, lock, name_prefix)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The style checker is catching that name_prefix is not defined here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, sorry, I deleted a line by mistake yesterday, when I have added the inline comment -_-'

@jhamman
Copy link
Member

jhamman commented Dec 9, 2020

Other than a small comment above, this looks good to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants