Skip to content

fix: honor storage_options in save and save_group - #4415

Merged
d-v-b merged 4 commits into
zarr-developers:mainfrom
Rodrigo-Palma:fix/save-storage-options
Sep 29, 2026
Merged

d-v-b merged 4 commits into
zarr-developers:mainfrom
Rodrigo-Palma:fix/save-storage-options

Conversation

@Rodrigo-Palma

Copy link
Copy Markdown
Contributor

Summary

save() never declared storage_options, so it arrived through **kwargs. In save(), **kwargs are the arrays to write, which caused two separate problems.

First, save(store, arr, storage_options=...) no longer took the single-array branch, because that branch requires len(kwargs) == 0. A lone array was written as a group holding arr_0 instead of as an array.

Second, the option never reached the store, so an fsspec URL was opened without it.

This declares storage_options on both the sync and the async save, forwards it to save_array and save_group, and documents it in the same words the sibling functions use.

One line was also removed from save_group: it forwarded storage_options to the per-array save_array calls for positional arrays only (keyword arrays never got it). By that point store_path has already consumed the options, so forwarding them again makes make_store_path reject them as unused. That is the TypeError in the second test below.

For reviewers

The removal inside save_group is the part worth a second look. Everything else is additive.

Measured on main, both new tests fail:

tests/test_api.py::test_save_storage_options_is_not_an_array
E   assert False                      # the node came back as a Group, not an Array

tests/test_api.py::test_save_group_storage_options_positional_args
E   TypeError: 'storage_options' was provided but unused. 'storage_options' is only
    used when the store is passed as an FSSpec URI string.        (_common.py:471)

On this branch tests/test_api.py is 355 passed, 9 skipped (CuPy not installed), and ruff check, ruff format --check and mypy are clean on the touched files.

Author attestation

  • I am a human, these are my changes, and I have reviewed and understood every change and can explain why each is correct.

TODO

  • Add unit tests and/or doctests in docstrings
  • Add docstrings and API docs for any new/modified user-facing classes and functions
  • New/modified features documented in docs/user-guide/*.md (bug fix, no user-guide change)
  • Changes documented as a new file in changes/
  • GitHub Actions have all passed
  • Test coverage is 100% (Codecov passes)

`save` did not declare `storage_options`, so it landed in `**kwargs` and was
counted as one of the arrays to save. Passing it turned a single array into a
group holding `arr_0`, and the options never reached the store.

`save_group` resolved the store itself and then passed `storage_options` on to
`save_array` for positional arrays, where `make_store_path` rejects them as
unused. The keyword branch right below it already did the right thing.
@codecov

codecov Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.43%. Comparing base (0ba2ca2) to head (475dff7).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4415   +/-   ##
=======================================
  Coverage   94.43%   94.43%           
=======================================
  Files          93       93           
  Lines       13219    13219           
=======================================
  Hits        12483    12483           
  Misses        736      736           
Files with missing lines Coverage Δ
src/zarr/api/asynchronous.py 96.32% <100.00%> (ø)
src/zarr/api/synchronous.py 92.95% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

An fsspec URL for a sync filesystem is opened through AsyncFileSystemWrapper,
which landed in fsspec 2024.12.0, and the min-deps CI job pins an older one.
Same guard the fsspec store tests already use.
@d-v-b d-v-b modified the milestone: 3.4.1 Sep 29, 2026
Assisted-by: ClaudeCode:claude-opus-5-5
@d-v-b d-v-b added this to the 3.4.1 milestone Sep 29, 2026
@d-v-b

d-v-b commented Sep 29, 2026

Copy link
Copy Markdown
Contributor

thanks @Rodrigo-Palma

@d-v-b
d-v-b enabled auto-merge (squash) September 29, 2026 15:23
@d-v-b
d-v-b merged commit 319bfa4 into zarr-developers:main Sep 29, 2026
39 checks passed
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.

2 participants