From 841df356e66abb12364346c91bce837f822d5329 Mon Sep 17 00:00:00 2001 From: brokkoli71 Date: Fri, 25 Apr 2025 16:28:00 +0200 Subject: [PATCH 1/9] move for LocalStore --- src/zarr/abc/store.py | 7 +++++++ src/zarr/storage/_local.py | 15 +++++++++++++++ tests/test_store/test_local.py | 18 ++++++++++++++++++ tests/test_store/test_memory.py | 6 ++++++ 4 files changed, 46 insertions(+) diff --git a/src/zarr/abc/store.py b/src/zarr/abc/store.py index 96165f8ba0..40ea91a8c7 100644 --- a/src/zarr/abc/store.py +++ b/src/zarr/abc/store.py @@ -12,6 +12,7 @@ if TYPE_CHECKING: from collections.abc import AsyncGenerator, AsyncIterator, Iterable + from pathlib import Path from types import TracebackType from typing import Any, Self, TypeAlias @@ -136,6 +137,12 @@ async def is_empty(self, prefix: str) -> bool: return False return True + async def move(self, path: Path | str) -> None: + """ + Move the store to another path + """ + raise NotImplementedError(f"store.move is not valid for {self.__class__.__name__}") + async def clear(self) -> None: """ Clear the store. diff --git a/src/zarr/storage/_local.py b/src/zarr/storage/_local.py index 85d244f17b..d63fccfc75 100644 --- a/src/zarr/storage/_local.py +++ b/src/zarr/storage/_local.py @@ -253,5 +253,20 @@ async def list_dir(self, prefix: str) -> AsyncIterator[str]: except (FileNotFoundError, NotADirectoryError): pass + async def move(self, path: Path | str) -> None: + # docstring inherited + + if isinstance(path, str): + path = Path(path) + if not isinstance(path, Path): + raise TypeError( + f"'path' must be a string or Path instance. Got an instance of {type(path)} instead." + ) + os.makedirs(path, exist_ok=True) + for file_name in os.listdir(self.root): + shutil.move(os.path.join(self.root, file_name), path) + + self.root = path + async def getsize(self, key: str) -> int: return os.path.getsize(self.root / key) diff --git a/tests/test_store/test_local.py b/tests/test_store/test_local.py index d9d941c6f0..a6600accf5 100644 --- a/tests/test_store/test_local.py +++ b/tests/test_store/test_local.py @@ -1,10 +1,13 @@ from __future__ import annotations +import os from typing import TYPE_CHECKING +import numpy as np import pytest import zarr +from zarr import create_array from zarr.core.buffer import Buffer, cpu from zarr.storage import LocalStore from zarr.testing.store import StoreTests @@ -74,3 +77,18 @@ async def test_get_with_prototype_default(self, store: LocalStore): await self.set(store, key, data_buf) observed = await store.get(key, prototype=None) assert_bytes_equal(observed, data_buf) + + async def test_move(self, tmp_path: pathlib.Path): + origin = tmp_path / "origin" + destination = tmp_path / "destintion" + + store = await LocalStore.open(root=origin) + array = create_array(store, data=np.arange(10)) + + await store.move(destination) + + assert store.root == destination + assert destination.exists() + assert origin.exists() + assert len(os.listdir(origin)) == 0 + assert np.array_equal(array[...], np.arange(10)) diff --git a/tests/test_store/test_memory.py b/tests/test_store/test_memory.py index e520c7d054..ca601583d2 100644 --- a/tests/test_store/test_memory.py +++ b/tests/test_store/test_memory.py @@ -77,6 +77,12 @@ async def test_deterministic_size( np.testing.assert_array_equal(a[:3], 1) np.testing.assert_array_equal(a[3:], 0) + async def test_move(self, store: MemoryStore): + with pytest.raises( + NotImplementedError, match=re.escape("store.move is not valid for MemoryStore") + ): + await store.move("path") + # TODO: fix this warning @pytest.mark.filterwarnings("ignore:Unclosed client session:ResourceWarning") From 0ae0fda15ddaf89af2bfe0a131019c3efe3092cd Mon Sep 17 00:00:00 2001 From: brokkoli71 Date: Tue, 29 Apr 2025 11:37:54 +0200 Subject: [PATCH 2/9] move for ZipStore --- src/zarr/storage/_zip.py | 14 ++++++++++++++ tests/test_store/test_zip.py | 15 +++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/src/zarr/storage/_zip.py b/src/zarr/storage/_zip.py index f9eb8d8808..d6e701c7cd 100644 --- a/src/zarr/storage/_zip.py +++ b/src/zarr/storage/_zip.py @@ -1,6 +1,7 @@ from __future__ import annotations import os +import shutil import threading import time import zipfile @@ -288,3 +289,16 @@ async def list_dir(self, prefix: str) -> AsyncIterator[str]: if k not in seen: seen.add(k) yield k + + async def move(self, path: Path | str) -> None: + # docstring inherited + + if isinstance(path, str): + path = Path(path) + if not isinstance(path, Path): + raise TypeError( + f"'path' must be a string or Path instance. Got an instance of {type(path)} instead." + ) + os.makedirs(path, exist_ok=True) + shutil.move(self.path, path) + self.path = path diff --git a/tests/test_store/test_zip.py b/tests/test_store/test_zip.py index 0237258ab1..32351a6822 100644 --- a/tests/test_store/test_zip.py +++ b/tests/test_store/test_zip.py @@ -10,6 +10,7 @@ import pytest import zarr +from zarr import create_array from zarr.core.buffer import Buffer, cpu, default_buffer_prototype from zarr.storage import ZipStore from zarr.testing.store import StoreTests @@ -135,3 +136,17 @@ def test_externally_zipped_store(self, tmp_path: Path) -> None: zipped = zarr.open_group(ZipStore(zip_path, mode="r"), mode="r") assert list(zipped.keys()) == list(root.keys()) assert list(zipped["foo"].keys()) == list(root["foo"].keys()) + + async def test_move(self, tmp_path: Path): + origin = tmp_path / "origin" + destination = tmp_path / "destintion" + + store = await ZipStore.open(path=origin, mode="w") + array = create_array(store, data=np.arange(10)) + + await store.move(destination) + + assert store.path == destination + assert destination.exists() + assert not origin.exists() + assert np.array_equal(array[...], np.arange(10)) From 92eec5f2b3a6c6b4b8e6525e7c70f68ecc295f53 Mon Sep 17 00:00:00 2001 From: brokkoli71 Date: Tue, 29 Apr 2025 14:23:25 +0200 Subject: [PATCH 3/9] remove redundant check --- src/zarr/storage/_local.py | 4 ---- src/zarr/storage/_zip.py | 4 ---- tests/test_store/test_local.py | 2 +- tests/test_store/test_zip.py | 2 +- 4 files changed, 2 insertions(+), 10 deletions(-) diff --git a/src/zarr/storage/_local.py b/src/zarr/storage/_local.py index d63fccfc75..e21861e9f7 100644 --- a/src/zarr/storage/_local.py +++ b/src/zarr/storage/_local.py @@ -258,10 +258,6 @@ async def move(self, path: Path | str) -> None: if isinstance(path, str): path = Path(path) - if not isinstance(path, Path): - raise TypeError( - f"'path' must be a string or Path instance. Got an instance of {type(path)} instead." - ) os.makedirs(path, exist_ok=True) for file_name in os.listdir(self.root): shutil.move(os.path.join(self.root, file_name), path) diff --git a/src/zarr/storage/_zip.py b/src/zarr/storage/_zip.py index d6e701c7cd..5a36dea38d 100644 --- a/src/zarr/storage/_zip.py +++ b/src/zarr/storage/_zip.py @@ -295,10 +295,6 @@ async def move(self, path: Path | str) -> None: if isinstance(path, str): path = Path(path) - if not isinstance(path, Path): - raise TypeError( - f"'path' must be a string or Path instance. Got an instance of {type(path)} instead." - ) os.makedirs(path, exist_ok=True) shutil.move(self.path, path) self.path = path diff --git a/tests/test_store/test_local.py b/tests/test_store/test_local.py index a6600accf5..f079256cde 100644 --- a/tests/test_store/test_local.py +++ b/tests/test_store/test_local.py @@ -85,7 +85,7 @@ async def test_move(self, tmp_path: pathlib.Path): store = await LocalStore.open(root=origin) array = create_array(store, data=np.arange(10)) - await store.move(destination) + await store.move(str(destination)) assert store.root == destination assert destination.exists() diff --git a/tests/test_store/test_zip.py b/tests/test_store/test_zip.py index 32351a6822..ca61e6d9e1 100644 --- a/tests/test_store/test_zip.py +++ b/tests/test_store/test_zip.py @@ -144,7 +144,7 @@ async def test_move(self, tmp_path: Path): store = await ZipStore.open(path=origin, mode="w") array = create_array(store, data=np.arange(10)) - await store.move(destination) + await store.move(str(destination)) assert store.path == destination assert destination.exists() From 2cd1482843a6ba36d8b9a886001ec96913b8429c Mon Sep 17 00:00:00 2001 From: brokkoli71 Date: Tue, 29 Apr 2025 14:29:24 +0200 Subject: [PATCH 4/9] open and close zipstore --- src/zarr/storage/_zip.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/zarr/storage/_zip.py b/src/zarr/storage/_zip.py index 5a36dea38d..6485a1f9f5 100644 --- a/src/zarr/storage/_zip.py +++ b/src/zarr/storage/_zip.py @@ -292,9 +292,10 @@ async def list_dir(self, prefix: str) -> AsyncIterator[str]: async def move(self, path: Path | str) -> None: # docstring inherited - if isinstance(path, str): path = Path(path) + self.close() os.makedirs(path, exist_ok=True) shutil.move(self.path, path) self.path = path + await self._open() From 12ab2fdd176f1443f7827b4b2917383ba05e0960 Mon Sep 17 00:00:00 2001 From: brokkoli71 Date: Tue, 29 Apr 2025 15:15:30 +0200 Subject: [PATCH 5/9] fix zipstore.move --- src/zarr/storage/_zip.py | 2 +- tests/test_store/test_zip.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/zarr/storage/_zip.py b/src/zarr/storage/_zip.py index 6485a1f9f5..c7ab659310 100644 --- a/src/zarr/storage/_zip.py +++ b/src/zarr/storage/_zip.py @@ -295,7 +295,7 @@ async def move(self, path: Path | str) -> None: if isinstance(path, str): path = Path(path) self.close() - os.makedirs(path, exist_ok=True) + os.makedirs(path.parent, exist_ok=True) shutil.move(self.path, path) self.path = path await self._open() diff --git a/tests/test_store/test_zip.py b/tests/test_store/test_zip.py index ca61e6d9e1..760fbe4a25 100644 --- a/tests/test_store/test_zip.py +++ b/tests/test_store/test_zip.py @@ -138,10 +138,10 @@ def test_externally_zipped_store(self, tmp_path: Path) -> None: assert list(zipped["foo"].keys()) == list(root["foo"].keys()) async def test_move(self, tmp_path: Path): - origin = tmp_path / "origin" - destination = tmp_path / "destintion" + origin = tmp_path / "origin.zip" + destination = tmp_path / "some_folder" / "destination.zip" - store = await ZipStore.open(path=origin, mode="w") + store = await ZipStore.open(path=origin, mode="a") array = create_array(store, data=np.arange(10)) await store.move(str(destination)) From 1fe39bb0799e3c069bba642d7ab4f673ed91da6a Mon Sep 17 00:00:00 2001 From: brokkoli71 Date: Fri, 2 May 2025 17:56:24 +0200 Subject: [PATCH 6/9] fix localstore.move for ndim>1 --- src/zarr/storage/_local.py | 20 ++++++++++++-------- tests/test_store/test_local.py | 15 ++++++++++----- 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/src/zarr/storage/_local.py b/src/zarr/storage/_local.py index e21861e9f7..aeb17716d3 100644 --- a/src/zarr/storage/_local.py +++ b/src/zarr/storage/_local.py @@ -253,16 +253,20 @@ async def list_dir(self, prefix: str) -> AsyncIterator[str]: except (FileNotFoundError, NotADirectoryError): pass - async def move(self, path: Path | str) -> None: + async def move(self, dest_root: Path | str) -> None: # docstring inherited - if isinstance(path, str): - path = Path(path) - os.makedirs(path, exist_ok=True) - for file_name in os.listdir(self.root): - shutil.move(os.path.join(self.root, file_name), path) - - self.root = path + if isinstance(dest_root, str): + dest_root = Path(dest_root) + os.makedirs(dest_root, exist_ok=True) + for src_file in self.root.rglob("*"): + if src_file.is_file(): + relative_path = src_file.relative_to(self.root) + dest_file_path = dest_root / relative_path + dest_file_path.parent.mkdir(parents=True, exist_ok=True) + shutil.move(str(src_file), str(dest_file_path)) + shutil.rmtree(self.root) + self.root = dest_root async def getsize(self, key: str) -> int: return os.path.getsize(self.root / key) diff --git a/tests/test_store/test_local.py b/tests/test_store/test_local.py index f079256cde..90ed2f5e4f 100644 --- a/tests/test_store/test_local.py +++ b/tests/test_store/test_local.py @@ -78,17 +78,22 @@ async def test_get_with_prototype_default(self, store: LocalStore): observed = await store.get(key, prototype=None) assert_bytes_equal(observed, data_buf) - async def test_move(self, tmp_path: pathlib.Path): + @pytest.mark.parametrize("ndim", [0, 1, 2, 3]) + async def test_move(self, tmp_path: pathlib.Path, ndim): origin = tmp_path / "origin" destination = tmp_path / "destintion" store = await LocalStore.open(root=origin) - array = create_array(store, data=np.arange(10)) + shape = (4,) * ndim + chunks = (2,) * ndim + data = np.arange(4**ndim) + if ndim>0: + data = data.reshape(*shape) + array = create_array(store, data=data, chunks=shape or "auto") await store.move(str(destination)) assert store.root == destination assert destination.exists() - assert origin.exists() - assert len(os.listdir(origin)) == 0 - assert np.array_equal(array[...], np.arange(10)) + assert not origin.exists() + assert np.array_equal(array[...], data) From 25d1c9d86404d77be0e600fe52df802da57b70e1 Mon Sep 17 00:00:00 2001 From: brokkoli71 Date: Fri, 2 May 2025 18:00:24 +0200 Subject: [PATCH 7/9] format --- tests/test_store/test_local.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/test_store/test_local.py b/tests/test_store/test_local.py index 90ed2f5e4f..0af990aecc 100644 --- a/tests/test_store/test_local.py +++ b/tests/test_store/test_local.py @@ -1,6 +1,5 @@ from __future__ import annotations -import os from typing import TYPE_CHECKING import numpy as np @@ -87,9 +86,9 @@ async def test_move(self, tmp_path: pathlib.Path, ndim): shape = (4,) * ndim chunks = (2,) * ndim data = np.arange(4**ndim) - if ndim>0: + if ndim > 0: data = data.reshape(*shape) - array = create_array(store, data=data, chunks=shape or "auto") + array = create_array(store, data=data, chunks=chunks or "auto") await store.move(str(destination)) From 2788467482615d2a9354d8c3552f2aca546dc56e Mon Sep 17 00:00:00 2001 From: brokkoli71 Date: Fri, 2 May 2025 18:04:49 +0200 Subject: [PATCH 8/9] remove abstract Store .move --- src/zarr/abc/store.py | 7 ------- src/zarr/storage/_local.py | 5 +++-- src/zarr/storage/_zip.py | 4 +++- tests/test_store/test_memory.py | 6 ------ 4 files changed, 6 insertions(+), 16 deletions(-) diff --git a/src/zarr/abc/store.py b/src/zarr/abc/store.py index 40ea91a8c7..96165f8ba0 100644 --- a/src/zarr/abc/store.py +++ b/src/zarr/abc/store.py @@ -12,7 +12,6 @@ if TYPE_CHECKING: from collections.abc import AsyncGenerator, AsyncIterator, Iterable - from pathlib import Path from types import TracebackType from typing import Any, Self, TypeAlias @@ -137,12 +136,6 @@ async def is_empty(self, prefix: str) -> bool: return False return True - async def move(self, path: Path | str) -> None: - """ - Move the store to another path - """ - raise NotImplementedError(f"store.move is not valid for {self.__class__.__name__}") - async def clear(self) -> None: """ Clear the store. diff --git a/src/zarr/storage/_local.py b/src/zarr/storage/_local.py index aeb17716d3..bf7c20702b 100644 --- a/src/zarr/storage/_local.py +++ b/src/zarr/storage/_local.py @@ -254,8 +254,9 @@ async def list_dir(self, prefix: str) -> AsyncIterator[str]: pass async def move(self, dest_root: Path | str) -> None: - # docstring inherited - + """ + Move the store to another path. The old root directory is deleted. + """ if isinstance(dest_root, str): dest_root = Path(dest_root) os.makedirs(dest_root, exist_ok=True) diff --git a/src/zarr/storage/_zip.py b/src/zarr/storage/_zip.py index c7ab659310..5d147deded 100644 --- a/src/zarr/storage/_zip.py +++ b/src/zarr/storage/_zip.py @@ -291,7 +291,9 @@ async def list_dir(self, prefix: str) -> AsyncIterator[str]: yield k async def move(self, path: Path | str) -> None: - # docstring inherited + """ + Move the store to another path. + """ if isinstance(path, str): path = Path(path) self.close() diff --git a/tests/test_store/test_memory.py b/tests/test_store/test_memory.py index ca601583d2..e520c7d054 100644 --- a/tests/test_store/test_memory.py +++ b/tests/test_store/test_memory.py @@ -77,12 +77,6 @@ async def test_deterministic_size( np.testing.assert_array_equal(a[:3], 1) np.testing.assert_array_equal(a[3:], 0) - async def test_move(self, store: MemoryStore): - with pytest.raises( - NotImplementedError, match=re.escape("store.move is not valid for MemoryStore") - ): - await store.move("path") - # TODO: fix this warning @pytest.mark.filterwarnings("ignore:Unclosed client session:ResourceWarning") From 6f07fe1fdce1d2b75a0c1fad230f923cfa8423dd Mon Sep 17 00:00:00 2001 From: brokkoli71 Date: Fri, 2 May 2025 18:13:21 +0200 Subject: [PATCH 9/9] document changes --- changes/3021.feature.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 changes/3021.feature.rst diff --git a/changes/3021.feature.rst b/changes/3021.feature.rst new file mode 100644 index 0000000000..8805797ce3 --- /dev/null +++ b/changes/3021.feature.rst @@ -0,0 +1 @@ +Implemented ``move`` for ``LocalStore`` and ``ZipStore``. This allows users to move the store to a different root path. \ No newline at end of file