diff --git a/changes/4415.bugfix.md b/changes/4415.bugfix.md new file mode 100644 index 0000000000..1b1777c9e3 --- /dev/null +++ b/changes/4415.bugfix.md @@ -0,0 +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. diff --git a/src/zarr/api/asynchronous.py b/src/zarr/api/asynchronous.py index 1fc10cdd1e..caca7b5888 100644 --- a/src/zarr/api/asynchronous.py +++ b/src/zarr/api/asynchronous.py @@ -435,6 +435,7 @@ async def save( *args: NDArrayLike, zarr_format: ZarrFormat | None = None, path: str | None = None, + storage_options: dict[str, Any] | None = None, **kwargs: Any, # TODO: type kwargs as valid args to save ) -> None: """Convenience function to save an array or group of arrays to the local file system. @@ -451,6 +452,9 @@ async def save( The zarr format to use when saving. path : str or None, optional The path within the group where the arrays will be saved. + storage_options : dict + If using an fsspec URL to create the store, these will be passed to + the backend implementation. Ignored otherwise. **kwargs NumPy arrays with data to save. """ @@ -458,9 +462,18 @@ async def save( if len(args) == 0 and len(kwargs) == 0: raise ValueError("at least one array must be provided") if len(args) == 1 and len(kwargs) == 0: - await save_array(store, args[0], zarr_format=zarr_format, path=path) + await save_array( + store, args[0], zarr_format=zarr_format, path=path, storage_options=storage_options + ) else: - await save_group(store, *args, zarr_format=zarr_format, path=path, **kwargs) + await save_group( + store, + *args, + zarr_format=zarr_format, + path=path, + storage_options=storage_options, + **kwargs, + ) async def save_array( @@ -566,16 +579,10 @@ async def save_group( if len(args) == 0 and len(kwargs) == 0: raise ValueError("at least one array must be provided") aws = [] + # `store_path` already consumed `storage_options`, so passing them on again would + # make `make_store_path` reject them as unused. for i, arr in enumerate(args): - aws.append( - save_array( - store_path, - arr, - zarr_format=zarr_format, - path=f"arr_{i}", - storage_options=storage_options, - ) - ) + aws.append(save_array(store_path, arr, zarr_format=zarr_format, path=f"arr_{i}")) for k, arr in kwargs.items(): aws.append(save_array(store_path, arr, zarr_format=zarr_format, path=k)) await asyncio.gather(*aws) diff --git a/src/zarr/api/synchronous.py b/src/zarr/api/synchronous.py index 6975f6d953..8d71239133 100644 --- a/src/zarr/api/synchronous.py +++ b/src/zarr/api/synchronous.py @@ -259,6 +259,7 @@ def save( *args: NDArrayLike, zarr_format: ZarrFormat | None = None, path: str | None = None, + storage_options: dict[str, Any] | None = None, **kwargs: Any, # TODO: type kwargs as valid args to async_api.save ) -> None: """Save an array or group of arrays to the local file system. @@ -275,10 +276,22 @@ def save( The zarr format to use when saving. path : str or None, optional The path within the group where the arrays will be saved. + storage_options : dict + If using an fsspec URL to create the store, these will be passed to + the backend implementation. Ignored otherwise. **kwargs NumPy arrays with data to save. """ - return sync(async_api.save(store, *args, zarr_format=zarr_format, path=path, **kwargs)) + return sync( + async_api.save( + store, + *args, + zarr_format=zarr_format, + path=path, + storage_options=storage_options, + **kwargs, + ) + ) def save_array( diff --git a/tests/test_api.py b/tests/test_api.py index 45d0c0dee4..16dc5bbffb 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -24,6 +24,7 @@ import numpy as np import pytest from numpy.testing import assert_array_equal +from packaging.version import parse as parse_version import zarr import zarr.api.asynchronous @@ -477,6 +478,32 @@ def test_save_errors() -> None: zarr.save("data/example.zarr", a, mode="w") +def test_save_storage_options_is_not_an_array(tmp_path: Path) -> None: + # `storage_options` is an option of `save`, not one of the arrays to save, so a + # single array still lands as an array and not as a group holding `arr_0`. + data = np.arange(10) + save(str(tmp_path / "a.zarr"), data, storage_options=None) + node = zarr.api.synchronous.open(str(tmp_path / "a.zarr")) + assert isinstance(node, Array) + assert_array_equal(node[:], data) + + +def test_save_group_storage_options_positional_args(tmp_path: Path) -> None: + # `storage_options` has to reach the store for positional arrays too, exactly as it + # does for keyword arrays. + fsspec = pytest.importorskip("fsspec") + # An fsspec URL for a sync filesystem needs AsyncFileSystemWrapper, which landed in + # fsspec 2024.12.0. The min-deps CI job pins an older one. + if parse_version(fsspec.__version__) < parse_version("2024.12.0"): + pytest.skip("No AsyncFileSystemWrapper") + data = np.arange(10) + url = f"local://{tmp_path}/group.zarr" + save_group(url, data, data, storage_options={"auto_mkdir": True}) + group = zarr.api.synchronous.open(str(tmp_path / "group.zarr")) + assert isinstance(group, Group) + assert sorted(group) == ["arr_0", "arr_1"] + + def test_open_with_mode_r(tmp_path: Path) -> None: # 'r' means read only (must exist) with pytest.raises(FileNotFoundError):