Storage options copy #221
Storage options copy #221
Conversation
The current implementation breaks when providing a single dict for `storage_options`, because `options.pop("chunks",...)` on line 238 removes the key `chunks` from the original `storage_options` dict after the first iteration. This happens because `options` is an alias of `storage_options` instead of a deep copy. My change fixes this and provides the intended behavior (all pyramid levels follow the same chunking scheme if `storage_options` is provided as a dict.
This fixes a zarr warning about this being ignored
There was a problem hiding this comment.
One API question about the da.to_zarr workflow. Otherwise, definitely valuable to have this fix tested and merged in preparation of a final release with Dask support.
I think the amended test covers the basic scenario passing a chunk size as a single storage_options dictionary. I suspect next action is to wait until we hear about #219 (comment) before merging?
Finally, re-reading #197 (review), I think we are still lacking the coverage of storage_options passed as a list of dictionaries. If it cannot easily be added in this PR, I'd propose to at least capture this as an issue so that we can come back to it.
will-moore
requested review from
joshmoore
and removed request for
joshmoore
|
@sbesson Added test for |
|
This PR is needed to fix the tests recently added to #220 |
This replaces #197, fixing merge conflicts between that and master and adding tests (described on the PR).
It also fixes an invalid comment and fixes a warning: