Skip to content

Update Zarr store creation functions#9790

Merged
mrocklin merged 1 commit intodask:mainfrom
rabernat:update-zarr-store-creation
Dec 30, 2022
Merged

Update Zarr store creation functions#9790
mrocklin merged 1 commit intodask:mainfrom
rabernat:update-zarr-store-creation

Conversation

@rabernat
Copy link
Copy Markdown
Contributor

  • Closes #xxxx
  • Tests added / passed
  • Passes pre-commit run --all-files

As discussed in zarr-developers/zarr-python#1309 (review), Zarr can now handle a the creation of more types of store object from URLs thanks to the FSStore class. This means that usually Dask can just pass through the store URL with no modifications (e.g. calling get_mapper). The only exception is when the user specifies storage_options explicitly.

@GPUtester
Copy link
Copy Markdown
Collaborator

Can one of the admins verify this patch?

Admins can comment ok to test to allow this one PR to run or add to allowlist to allow all future PRs from the same author to run.

@github-actions github-actions bot added the array label Dec 28, 2022
@mrocklin
Copy link
Copy Markdown
Member

Cool. Happy to see things moved upstream. Is this new functionality widely deployed? Should we be concerned at all about backwards compatibility? (how old is it / what fraction of users are likely using that version or older?)

@ncclementi
Copy link
Copy Markdown
Member

add to allowlist

@mrocklin
Copy link
Copy Markdown
Member

CI failure seems unrelated. Testing main here: #9792

@rabernat
Copy link
Copy Markdown
Contributor Author

Is this new functionality widely deployed? Should we be concerned at all about backwards compatibility? (how old is it / what fraction of users are likely using that version or older?)

This should work with all versions of Zarr > 2.6.0, which was released in July, 2021. That feels like a long time ago to me, but I have no data on what fraction of users are using older versions. (Do you have some way to get those numbers?)

@mrocklin
Copy link
Copy Markdown
Member

Makes sense. I was just curious. CI seems unhappy. I suspect due to unrelated issues. I'm going to pause on this for a moment while I check those out. My apologies for the delay.

@mrocklin mrocklin merged commit 0cd60ee into dask:main Dec 30, 2022
@mrocklin
Copy link
Copy Markdown
Member

Thanks @rabernat . This is in.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants