-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Remove special mapping of auto to {} in open_zarr
#11010
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
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: Justus Magin <[email protected]>
| from_array_kwargs = {} | ||
|
|
||
| if chunks == "auto": | ||
| if chunks is _default: |
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.
let's issue a DeprecationWarning saying the default will switch to chunks=None to match open_dataset. If they want the current behaviour with dask et al, users should pass in chunks={}
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.
Oh I was actually thinking that we not do that part. The issue that we are trying to fix with this PR is really that chunks="auto" means different things in open_zarr and open_dataset(,,, engine="zarr"). That was the part that felt deeply surprising to me and @norlandrhagen. As long as we fix that I don't think we need to change the default value.
xr.open_zarrandxr.open_dataset(..., engine='zarr')withchunks="auto"#11002whats-new.rstThis PR makes the handling of
chunks="auto"consistent betweenopen_zarrandopen_dataset(..., engine="zarr").The handling of chunks still differs in
open_zarrvsopen_dataset(..., engine="zarr")in that the default inopen_zarris to usechunks={}and a chunk manager (aka dask) when available in your env. And inopen_datasetthe default is to usechunks=None(aka no chunks).