Skip to content

Commit 319bfa4

Browse files
Rodrigo-Palmad-v-b
andauthored
fix: honor storage_options in save and save_group (#4415)
* fix: honor storage_options in save and save_group `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. * test: skip the fsspec URL case on the min-deps job 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. * docs: rename changelog fragment to the PR number Assisted-by: ClaudeCode:claude-opus-5-5 --------- Co-authored-by: Davis Vann Bennett <davis.v.bennett@gmail.com>
1 parent 0ba2ca2 commit 319bfa4

4 files changed

Lines changed: 60 additions & 12 deletions

File tree

‎changes/4415.bugfix.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
`zarr.save` now takes `storage_options` as an option instead of treating it as one of the arrays to save: saving a single array with `storage_options` set no longer produces a group holding `arr_0`, and the options now reach the store. `zarr.save_group` no longer fails with `'storage_options' was provided but unused` when the arrays are passed positionally.

‎src/zarr/api/asynchronous.py‎

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -435,6 +435,7 @@ async def save(
435435
*args: NDArrayLike,
436436
zarr_format: ZarrFormat | None = None,
437437
path: str | None = None,
438+
storage_options: dict[str, Any] | None = None,
438439
**kwargs: Any, # TODO: type kwargs as valid args to save
439440
) -> None:
440441
"""Convenience function to save an array or group of arrays to the local file system.
@@ -451,16 +452,28 @@ async def save(
451452
The zarr format to use when saving.
452453
path : str or None, optional
453454
The path within the group where the arrays will be saved.
455+
storage_options : dict
456+
If using an fsspec URL to create the store, these will be passed to
457+
the backend implementation. Ignored otherwise.
454458
**kwargs
455459
NumPy arrays with data to save.
456460
"""
457461

458462
if len(args) == 0 and len(kwargs) == 0:
459463
raise ValueError("at least one array must be provided")
460464
if len(args) == 1 and len(kwargs) == 0:
461-
await save_array(store, args[0], zarr_format=zarr_format, path=path)
465+
await save_array(
466+
store, args[0], zarr_format=zarr_format, path=path, storage_options=storage_options
467+
)
462468
else:
463-
await save_group(store, *args, zarr_format=zarr_format, path=path, **kwargs)
469+
await save_group(
470+
store,
471+
*args,
472+
zarr_format=zarr_format,
473+
path=path,
474+
storage_options=storage_options,
475+
**kwargs,
476+
)
464477

465478

466479
async def save_array(
@@ -566,16 +579,10 @@ async def save_group(
566579
if len(args) == 0 and len(kwargs) == 0:
567580
raise ValueError("at least one array must be provided")
568581
aws = []
582+
# `store_path` already consumed `storage_options`, so passing them on again would
583+
# make `make_store_path` reject them as unused.
569584
for i, arr in enumerate(args):
570-
aws.append(
571-
save_array(
572-
store_path,
573-
arr,
574-
zarr_format=zarr_format,
575-
path=f"arr_{i}",
576-
storage_options=storage_options,
577-
)
578-
)
585+
aws.append(save_array(store_path, arr, zarr_format=zarr_format, path=f"arr_{i}"))
579586
for k, arr in kwargs.items():
580587
aws.append(save_array(store_path, arr, zarr_format=zarr_format, path=k))
581588
await asyncio.gather(*aws)

‎src/zarr/api/synchronous.py‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,7 @@ def save(
259259
*args: NDArrayLike,
260260
zarr_format: ZarrFormat | None = None,
261261
path: str | None = None,
262+
storage_options: dict[str, Any] | None = None,
262263
**kwargs: Any, # TODO: type kwargs as valid args to async_api.save
263264
) -> None:
264265
"""Save an array or group of arrays to the local file system.
@@ -275,10 +276,22 @@ def save(
275276
The zarr format to use when saving.
276277
path : str or None, optional
277278
The path within the group where the arrays will be saved.
279+
storage_options : dict
280+
If using an fsspec URL to create the store, these will be passed to
281+
the backend implementation. Ignored otherwise.
278282
**kwargs
279283
NumPy arrays with data to save.
280284
"""
281-
return sync(async_api.save(store, *args, zarr_format=zarr_format, path=path, **kwargs))
285+
return sync(
286+
async_api.save(
287+
store,
288+
*args,
289+
zarr_format=zarr_format,
290+
path=path,
291+
storage_options=storage_options,
292+
**kwargs,
293+
)
294+
)
282295

283296

284297
def save_array(

‎tests/test_api.py‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import numpy as np
2525
import pytest
2626
from numpy.testing import assert_array_equal
27+
from packaging.version import parse as parse_version
2728

2829
import zarr
2930
import zarr.api.asynchronous
@@ -477,6 +478,32 @@ def test_save_errors() -> None:
477478
zarr.save("data/example.zarr", a, mode="w")
478479

479480

481+
def test_save_storage_options_is_not_an_array(tmp_path: Path) -> None:
482+
# `storage_options` is an option of `save`, not one of the arrays to save, so a
483+
# single array still lands as an array and not as a group holding `arr_0`.
484+
data = np.arange(10)
485+
save(str(tmp_path / "a.zarr"), data, storage_options=None)
486+
node = zarr.api.synchronous.open(str(tmp_path / "a.zarr"))
487+
assert isinstance(node, Array)
488+
assert_array_equal(node[:], data)
489+
490+
491+
def test_save_group_storage_options_positional_args(tmp_path: Path) -> None:
492+
# `storage_options` has to reach the store for positional arrays too, exactly as it
493+
# does for keyword arrays.
494+
fsspec = pytest.importorskip("fsspec")
495+
# An fsspec URL for a sync filesystem needs AsyncFileSystemWrapper, which landed in
496+
# fsspec 2024.12.0. The min-deps CI job pins an older one.
497+
if parse_version(fsspec.__version__) < parse_version("2024.12.0"):
498+
pytest.skip("No AsyncFileSystemWrapper")
499+
data = np.arange(10)
500+
url = f"local://{tmp_path}/group.zarr"
501+
save_group(url, data, data, storage_options={"auto_mkdir": True})
502+
group = zarr.api.synchronous.open(str(tmp_path / "group.zarr"))
503+
assert isinstance(group, Group)
504+
assert sorted(group) == ["arr_0", "arr_1"]
505+
506+
480507
def test_open_with_mode_r(tmp_path: Path) -> None:
481508
# 'r' means read only (must exist)
482509
with pytest.raises(FileNotFoundError):

0 commit comments

Comments
 (0)