From 4863811d99b1c7d2449287ef1fe6009c91b69b9c Mon Sep 17 00:00:00 2001 From: brokkoli71 Date: Tue, 3 Jun 2025 16:25:45 +0200 Subject: [PATCH 1/4] test and fix path argument in save_group --- src/zarr/api/asynchronous.py | 3 +-- tests/test_api.py | 13 +++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/zarr/api/asynchronous.py b/src/zarr/api/asynchronous.py index 7e10350bb9..b296d21bd4 100644 --- a/src/zarr/api/asynchronous.py +++ b/src/zarr/api/asynchronous.py @@ -505,13 +505,12 @@ async def save_group( raise ValueError("at least one array must be provided") aws = [] for i, arr in enumerate(args): - _path = f"{path}/arr_{i}" if path is not None else f"arr_{i}" aws.append( save_array( store_path, arr, zarr_format=zarr_format, - path=_path, + path=f"arr_{i}", storage_options=storage_options, ) ) diff --git a/tests/test_api.py b/tests/test_api.py index 8cd4ab6b60..dceca0716e 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -229,22 +229,23 @@ async def test_open_group_unspecified_version( @pytest.mark.parametrize("store", ["local", "memory", "zip"], indirect=["store"]) @pytest.mark.parametrize("n_args", [10, 1, 0]) @pytest.mark.parametrize("n_kwargs", [10, 1, 0]) -def test_save(store: Store, n_args: int, n_kwargs: int) -> None: +@pytest.mark.parametrize("path", [None, "some_path"]) +def test_save(store: Store, n_args: int, n_kwargs: int, path) -> None: data = np.arange(10) args = [np.arange(10) for _ in range(n_args)] kwargs = {f"arg_{i}": data for i in range(n_kwargs)} if n_kwargs == 0 and n_args == 0: with pytest.raises(ValueError): - save(store) + save(store, path=path) elif n_args == 1 and n_kwargs == 0: - save(store, *args) - array = zarr.api.synchronous.open(store) + save(store, *args, path=path) + array = zarr.api.synchronous.open(store, path=path) assert isinstance(array, Array) assert_array_equal(array[:], data) else: - save(store, *args, **kwargs) # type: ignore [arg-type] - group = zarr.api.synchronous.open(store) + save(store, *args, path=path, **kwargs) # type: ignore [arg-type] + group = zarr.api.synchronous.open(store, path=path) assert isinstance(group, Group) for array in group.array_values(): assert_array_equal(array[:], data) From 2d0f6e9ce9de44bfbb5a9c4ed54d2b31f7de1b10 Mon Sep 17 00:00:00 2001 From: brokkoli71 Date: Tue, 10 Jun 2025 10:20:51 +0200 Subject: [PATCH 2/4] add tests for load_array --- tests/test_api.py | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/tests/test_api.py b/tests/test_api.py index dceca0716e..e13bf61c9c 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -39,7 +39,7 @@ ) from zarr.core.buffer import NDArrayLike from zarr.errors import MetadataValidationError -from zarr.storage import MemoryStore +from zarr.storage import MemoryStore, ZipStore from zarr.storage._utils import normalize_path from zarr.testing.utils import gpu_test @@ -230,7 +230,7 @@ async def test_open_group_unspecified_version( @pytest.mark.parametrize("n_args", [10, 1, 0]) @pytest.mark.parametrize("n_kwargs", [10, 1, 0]) @pytest.mark.parametrize("path", [None, "some_path"]) -def test_save(store: Store, n_args: int, n_kwargs: int, path) -> None: +def test_save(store: Store, n_args: int, n_kwargs: int, path: None | str) -> None: data = np.arange(10) args = [np.arange(10) for _ in range(n_args)] kwargs = {f"arg_{i}": data for i in range(n_kwargs)} @@ -385,8 +385,8 @@ def test_array_order_warns(order: MemoryOrder | None, zarr_format: ZarrFormat) - # assert "LazyLoader: " in repr(loader) -def test_load_array(memory_store: Store) -> None: - store = memory_store +def test_load_array(sync_store: Store) -> None: + store = sync_store foo = np.arange(100) bar = np.arange(100, 0, -1) save(store, foo=foo, bar=bar) @@ -400,6 +400,17 @@ def test_load_array(memory_store: Store) -> None: else: assert_array_equal(bar, array) +def test_load_zip(tmp_path: pathlib.Path) -> None: + file = tmp_path / "test.zip" + # Create a zip file with some data + with ZipStore(file, mode="w", read_only=False) as zs: + save(zs, np.arange(100).reshape(10, 10), path="data") + with ZipStore(file, mode="r", read_only=False) as zs: + data = zarr.load(store=zs, path="data") # This works + print(data.shape) + with ZipStore(file, mode="r") as zs: + data = zarr.load(store=zs, path="data") # This does not work + print(data.shape) def test_tree() -> None: pytest.importorskip("rich") From 7c0345c1ab1edd95d9d49401af9807d4049a9217 Mon Sep 17 00:00:00 2001 From: brokkoli71 Date: Tue, 10 Jun 2025 10:22:51 +0200 Subject: [PATCH 3/4] Revert "add tests for load_array" partially This reverts commit 2d0f6e9ce9de44bfbb5a9c4ed54d2b31f7de1b10 partially. --- tests/test_api.py | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/tests/test_api.py b/tests/test_api.py index e13bf61c9c..5d41a37493 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -39,7 +39,7 @@ ) from zarr.core.buffer import NDArrayLike from zarr.errors import MetadataValidationError -from zarr.storage import MemoryStore, ZipStore +from zarr.storage import MemoryStore from zarr.storage._utils import normalize_path from zarr.testing.utils import gpu_test @@ -400,17 +400,6 @@ def test_load_array(sync_store: Store) -> None: else: assert_array_equal(bar, array) -def test_load_zip(tmp_path: pathlib.Path) -> None: - file = tmp_path / "test.zip" - # Create a zip file with some data - with ZipStore(file, mode="w", read_only=False) as zs: - save(zs, np.arange(100).reshape(10, 10), path="data") - with ZipStore(file, mode="r", read_only=False) as zs: - data = zarr.load(store=zs, path="data") # This works - print(data.shape) - with ZipStore(file, mode="r") as zs: - data = zarr.load(store=zs, path="data") # This does not work - print(data.shape) def test_tree() -> None: pytest.importorskip("rich") From ce942fb8678434e21329de7e87fe948b7efa46d3 Mon Sep 17 00:00:00 2001 From: brokkoli71 Date: Tue, 10 Jun 2025 10:29:42 +0200 Subject: [PATCH 4/4] document changes --- changes/3127.bugfix.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 changes/3127.bugfix.rst diff --git a/changes/3127.bugfix.rst b/changes/3127.bugfix.rst new file mode 100644 index 0000000000..35d7f5d329 --- /dev/null +++ b/changes/3127.bugfix.rst @@ -0,0 +1,2 @@ +When `zarr.save` has an argument `path=some/path/` and multiple arrays in `args`, the path resulted in `some/path/some/path` due to using the `path` +argument twice while building the array path. This is now fixed. \ No newline at end of file