From 5c67734f6cfc03ae90edc3ee0713bc1be427e6cc Mon Sep 17 00:00:00 2001 From: Stephan Hoyer Date: Thu, 28 Aug 2025 10:55:57 -0700 Subject: [PATCH 1/3] Remove Store.set_partial_writes This feature was unused by the rest of Zarr-Python, and was only implemented for LocalStore and stores that wrap other stores. The Zarr v3 spec still mentions partial writes, so it should probably also be updated. --- changes/2859.removal.rst | 2 ++ src/zarr/abc/store.py | 25 ++++++------------------ src/zarr/storage/_common.py | 11 +---------- src/zarr/storage/_fsspec.py | 6 ------ src/zarr/storage/_local.py | 33 ++++---------------------------- src/zarr/storage/_logging.py | 13 ------------- src/zarr/storage/_memory.py | 8 -------- src/zarr/storage/_obstore.py | 11 ----------- src/zarr/storage/_wrapper.py | 9 --------- src/zarr/storage/_zip.py | 7 ------- src/zarr/testing/stateful.py | 7 ------- src/zarr/testing/store.py | 4 ++-- tests/test_store/test_fsspec.py | 3 --- tests/test_store/test_local.py | 3 --- tests/test_store/test_logging.py | 3 --- tests/test_store/test_memory.py | 6 ------ tests/test_store/test_object.py | 5 ----- tests/test_store/test_wrapper.py | 3 --- tests/test_store/test_zip.py | 3 --- 19 files changed, 15 insertions(+), 147 deletions(-) create mode 100644 changes/2859.removal.rst diff --git a/changes/2859.removal.rst b/changes/2859.removal.rst new file mode 100644 index 0000000000..bd417855f3 --- /dev/null +++ b/changes/2859.removal.rst @@ -0,0 +1,2 @@ +The ``Store.set_partial_writes`` method, which was not used by Zarr-Python, has been removed. +``store.supports_partial_writes`` is now always ``False``. diff --git a/src/zarr/abc/store.py b/src/zarr/abc/store.py index 53e981c3bd..c94ecfdd4b 100644 --- a/src/zarr/abc/store.py +++ b/src/zarr/abc/store.py @@ -4,7 +4,7 @@ from asyncio import gather from dataclasses import dataclass from itertools import starmap -from typing import TYPE_CHECKING, Protocol, runtime_checkable +from typing import TYPE_CHECKING, Literal, Protocol, runtime_checkable if TYPE_CHECKING: from collections.abc import AsyncGenerator, AsyncIterator, Iterable @@ -310,25 +310,12 @@ async def delete(self, key: str) -> None: ... @property - @abstractmethod - def supports_partial_writes(self) -> bool: - """Does the store support partial writes?""" - ... - - @abstractmethod - async def set_partial_values( - self, key_start_values: Iterable[tuple[str, int, BytesLike]] - ) -> None: - """Store values at a given key, starting at byte range_start. + def supports_partial_writes(self) -> Literal[False]: + """Does the store support partial writes? - Parameters - ---------- - key_start_values : list[tuple[str, int, BytesLike]] - set of key, range_start, values triples, a key may occur multiple times with different - range_starts, range_starts (considering the length of the respective values) must not - specify overlapping ranges for the same key + Partial writes are no longer used by Zarr, so this is always false. """ - ... + return False @property @abstractmethod @@ -499,7 +486,7 @@ async def get( self, prototype: BufferPrototype, byte_range: ByteRequest | None = None ) -> Buffer | None: ... - async def set(self, value: Buffer, byte_range: ByteRequest | None = None) -> None: ... + async def set(self, value: Buffer) -> None: ... async def delete(self) -> None: ... diff --git a/src/zarr/storage/_common.py b/src/zarr/storage/_common.py index 66a53c19df..ff757f9a99 100644 --- a/src/zarr/storage/_common.py +++ b/src/zarr/storage/_common.py @@ -163,7 +163,7 @@ async def get( prototype = default_buffer_prototype() return await self.store.get(self.path, prototype=prototype, byte_range=byte_range) - async def set(self, value: Buffer, byte_range: ByteRequest | None = None) -> None: + async def set(self, value: Buffer) -> None: """ Write bytes to the store. @@ -171,16 +171,7 @@ async def set(self, value: Buffer, byte_range: ByteRequest | None = None) -> Non ---------- value : Buffer The buffer to write. - byte_range : ByteRequest, optional - The range of bytes to write. If None, the entire buffer is written. - - Raises - ------ - NotImplementedError - If `byte_range` is not None, because Store.set does not support partial writes yet. """ - if byte_range is not None: - raise NotImplementedError("Store.set does not have partial writes yet") await self.store.set(self.path, value) async def delete(self) -> None: diff --git a/src/zarr/storage/_fsspec.py b/src/zarr/storage/_fsspec.py index bbb934cc7d..cb6b4f55ed 100644 --- a/src/zarr/storage/_fsspec.py +++ b/src/zarr/storage/_fsspec.py @@ -418,12 +418,6 @@ async def get_partial_values( return [None if isinstance(r, Exception) else prototype.buffer.from_bytes(r) for r in res] - async def set_partial_values( - self, key_start_values: Iterable[tuple[str, int, BytesLike]] - ) -> None: - # docstring inherited - raise NotImplementedError - async def list(self) -> AsyncIterator[str]: # docstring inherited allfiles = await self.fs._find(self.path, detail=False, withdirs=False) diff --git a/src/zarr/storage/_local.py b/src/zarr/storage/_local.py index 717ed04144..f64da71bb4 100644 --- a/src/zarr/storage/_local.py +++ b/src/zarr/storage/_local.py @@ -77,23 +77,12 @@ def _atomic_write( raise -def _put( - path: Path, - value: Buffer, - start: int | None = None, - exclusive: bool = False, -) -> int | None: +def _put(path: Path, value: Buffer, exclusive: bool = False) -> int: path.parent.mkdir(parents=True, exist_ok=True) # write takes any object supporting the buffer protocol view = value.as_buffer_like() - if start is not None: - with path.open("r+b") as f: - f.seek(start) - f.write(view) - return None - else: - with _atomic_write(path, "wb", exclusive=exclusive) as f: - return f.write(view) + with _atomic_write(path, "wb", exclusive=exclusive) as f: + return f.write(view) class LocalStore(Store): @@ -111,14 +100,12 @@ class LocalStore(Store): ---------- supports_writes supports_deletes - supports_partial_writes supports_listing root """ supports_writes: bool = True supports_deletes: bool = True - supports_partial_writes: bool = True supports_listing: bool = True root: Path @@ -253,19 +240,7 @@ async def _set(self, key: str, value: Buffer, exclusive: bool = False) -> None: f"LocalStore.set(): `value` must be a Buffer instance. Got an instance of {type(value)} instead." ) path = self.root / key - await asyncio.to_thread(_put, path, value, start=None, exclusive=exclusive) - - async def set_partial_values( - self, key_start_values: Iterable[tuple[str, int, bytes | bytearray | memoryview]] - ) -> None: - # docstring inherited - self._check_writable() - args = [] - for key, start, value in key_start_values: - assert isinstance(key, str) - path = self.root / key - args.append((_put, path, value, start)) - await concurrent_map(args, asyncio.to_thread, limit=None) # TODO: fix limit + await asyncio.to_thread(_put, path, value, exclusive=exclusive) async def delete(self, key: str) -> None: """ diff --git a/src/zarr/storage/_logging.py b/src/zarr/storage/_logging.py index a2164a418f..dd20d49ae5 100644 --- a/src/zarr/storage/_logging.py +++ b/src/zarr/storage/_logging.py @@ -115,11 +115,6 @@ def supports_deletes(self) -> bool: with self.log(): return self._store.supports_deletes - @property - def supports_partial_writes(self) -> bool: - with self.log(): - return self._store.supports_partial_writes - @property def supports_listing(self) -> bool: with self.log(): @@ -207,14 +202,6 @@ async def delete(self, key: str) -> None: with self.log(key): return await self._store.delete(key=key) - async def set_partial_values( - self, key_start_values: Iterable[tuple[str, int, bytes | bytearray | memoryview]] - ) -> None: - # docstring inherited - keys = ",".join([k[0] for k in key_start_values]) - with self.log(keys): - return await self._store.set_partial_values(key_start_values=key_start_values) - async def list(self) -> AsyncGenerator[str, None]: # docstring inherited with self.log(): diff --git a/src/zarr/storage/_memory.py b/src/zarr/storage/_memory.py index 5c12563136..e6076d9669 100644 --- a/src/zarr/storage/_memory.py +++ b/src/zarr/storage/_memory.py @@ -32,13 +32,11 @@ class MemoryStore(Store): ---------- supports_writes supports_deletes - supports_partial_writes supports_listing """ supports_writes: bool = True supports_deletes: bool = True - supports_partial_writes: bool = True supports_listing: bool = True _store_dict: MutableMapping[str, Buffer] @@ -143,12 +141,6 @@ async def delete(self, key: str) -> None: except KeyError: logger.debug("Key %s does not exist.", key) - async def set_partial_values( - self, key_start_values: Iterable[tuple[str, int, bytes | bytearray | memoryview[int]]] - ) -> None: - # docstring inherited - raise NotImplementedError - async def list(self) -> AsyncIterator[str]: # docstring inherited for key in self._store_dict: diff --git a/src/zarr/storage/_obstore.py b/src/zarr/storage/_obstore.py index e1469a991e..6bf60655c6 100644 --- a/src/zarr/storage/_obstore.py +++ b/src/zarr/storage/_obstore.py @@ -196,17 +196,6 @@ async def delete(self, key: str) -> None: with contextlib.suppress(FileNotFoundError): await obs.delete_async(self.store, key) - @property - def supports_partial_writes(self) -> bool: - # docstring inherited - return False - - async def set_partial_values( - self, key_start_values: Iterable[tuple[str, int, BytesLike]] - ) -> None: - # docstring inherited - raise NotImplementedError - @property def supports_listing(self) -> bool: # docstring inherited diff --git a/src/zarr/storage/_wrapper.py b/src/zarr/storage/_wrapper.py index f21d378191..dccdd35f8d 100644 --- a/src/zarr/storage/_wrapper.py +++ b/src/zarr/storage/_wrapper.py @@ -119,15 +119,6 @@ def supports_deletes(self) -> bool: async def delete(self, key: str) -> None: await self._store.delete(key) - @property - def supports_partial_writes(self) -> bool: - return self._store.supports_partial_writes - - async def set_partial_values( - self, key_start_values: Iterable[tuple[str, int, BytesLike]] - ) -> None: - return await self._store.set_partial_values(key_start_values) - @property def supports_listing(self) -> bool: return self._store.supports_listing diff --git a/src/zarr/storage/_zip.py b/src/zarr/storage/_zip.py index e52f160860..72bf9e335a 100644 --- a/src/zarr/storage/_zip.py +++ b/src/zarr/storage/_zip.py @@ -48,7 +48,6 @@ class ZipStore(Store): allowed_exceptions supports_writes supports_deletes - supports_partial_writes supports_listing path compression @@ -57,7 +56,6 @@ class ZipStore(Store): supports_writes: bool = True supports_deletes: bool = False - supports_partial_writes: bool = False supports_listing: bool = True path: Path @@ -222,11 +220,6 @@ async def set(self, key: str, value: Buffer) -> None: with self._lock: self._set(key, value) - async def set_partial_values( - self, key_start_values: Iterable[tuple[str, int, bytes | bytearray | memoryview[int]]] - ) -> None: - raise NotImplementedError - async def set_if_not_exists(self, key: str, value: Buffer) -> None: self._check_writable() with self._lock: diff --git a/src/zarr/testing/stateful.py b/src/zarr/testing/stateful.py index 8ada53a55a..c363c13983 100644 --- a/src/zarr/testing/stateful.py +++ b/src/zarr/testing/stateful.py @@ -467,17 +467,10 @@ def list_dir(self, prefix: str) -> None: def list_prefix(self, prefix: str) -> None: raise NotImplementedError - def set_partial_values(self, key_start_values: Any) -> None: - raise NotImplementedError - @property def supports_listing(self) -> bool: return self.store.supports_listing - @property - def supports_partial_writes(self) -> bool: - return self.supports_partial_writes - @property def supports_writes(self) -> bool: return self.store.supports_writes diff --git a/src/zarr/testing/store.py b/src/zarr/testing/store.py index d2946705f0..ad3b80da41 100644 --- a/src/zarr/testing/store.py +++ b/src/zarr/testing/store.py @@ -68,8 +68,8 @@ def test_store_repr(self, store: S) -> None: ... @abstractmethod def test_store_supports_writes(self, store: S) -> None: ... - @abstractmethod - def test_store_supports_partial_writes(self, store: S) -> None: ... + def test_store_supports_partial_writes(self, store: S) -> None: + assert not store.supports_partial_writes @abstractmethod def test_store_supports_listing(self, store: S) -> None: ... diff --git a/tests/test_store/test_fsspec.py b/tests/test_store/test_fsspec.py index 82a96b5d1e..e970c674d4 100644 --- a/tests/test_store/test_fsspec.py +++ b/tests/test_store/test_fsspec.py @@ -169,9 +169,6 @@ def test_store_repr(self, store: FsspecStore) -> None: def test_store_supports_writes(self, store: FsspecStore) -> None: assert store.supports_writes - def test_store_supports_partial_writes(self, store: FsspecStore) -> None: - assert not store.supports_partial_writes - def test_store_supports_listing(self, store: FsspecStore) -> None: assert store.supports_listing diff --git a/tests/test_store/test_local.py b/tests/test_store/test_local.py index 970bb7d374..d7cf8a75ff 100644 --- a/tests/test_store/test_local.py +++ b/tests/test_store/test_local.py @@ -38,9 +38,6 @@ def test_store_repr(self, store: LocalStore) -> None: def test_store_supports_writes(self, store: LocalStore) -> None: assert store.supports_writes - def test_store_supports_partial_writes(self, store: LocalStore) -> None: - assert store.supports_partial_writes - def test_store_supports_listing(self, store: LocalStore) -> None: assert store.supports_listing diff --git a/tests/test_store/test_logging.py b/tests/test_store/test_logging.py index 1a89dca874..d99ac5dc4f 100644 --- a/tests/test_store/test_logging.py +++ b/tests/test_store/test_logging.py @@ -44,9 +44,6 @@ def store(self, store_kwargs: str | dict[str, Buffer] | None) -> LoggingStore: def test_store_supports_writes(self, store: LoggingStore) -> None: assert store.supports_writes - def test_store_supports_partial_writes(self, store: LoggingStore) -> None: - assert store.supports_partial_writes - def test_store_supports_listing(self, store: LoggingStore) -> None: assert store.supports_listing diff --git a/tests/test_store/test_memory.py b/tests/test_store/test_memory.py index 0b6bae757d..29fa9b2964 100644 --- a/tests/test_store/test_memory.py +++ b/tests/test_store/test_memory.py @@ -54,9 +54,6 @@ def test_store_supports_writes(self, store: MemoryStore) -> None: def test_store_supports_listing(self, store: MemoryStore) -> None: assert store.supports_listing - def test_store_supports_partial_writes(self, store: MemoryStore) -> None: - assert store.supports_partial_writes - async def test_list_prefix(self, store: MemoryStore) -> None: assert True @@ -115,9 +112,6 @@ def test_store_supports_writes(self, store: GpuMemoryStore) -> None: def test_store_supports_listing(self, store: GpuMemoryStore) -> None: assert store.supports_listing - def test_store_supports_partial_writes(self, store: GpuMemoryStore) -> None: - assert store.supports_partial_writes - async def test_list_prefix(self, store: GpuMemoryStore) -> None: assert True diff --git a/tests/test_store/test_object.py b/tests/test_store/test_object.py index d8b89e56b7..3217069c2d 100644 --- a/tests/test_store/test_object.py +++ b/tests/test_store/test_object.py @@ -48,11 +48,6 @@ def test_store_repr(self, store: ObjectStore) -> None: def test_store_supports_writes(self, store: ObjectStore) -> None: assert store.supports_writes - async def test_store_supports_partial_writes(self, store: ObjectStore) -> None: - assert not store.supports_partial_writes - with pytest.raises(NotImplementedError): - await store.set_partial_values([("foo", 0, b"\x01\x02\x03\x04")]) - def test_store_supports_listing(self, store: ObjectStore) -> None: assert store.supports_listing diff --git a/tests/test_store/test_wrapper.py b/tests/test_store/test_wrapper.py index c6edd4f4dd..4478e1468f 100644 --- a/tests/test_store/test_wrapper.py +++ b/tests/test_store/test_wrapper.py @@ -43,9 +43,6 @@ def open_kwargs(self, tmpdir) -> dict[str, str]: def test_store_supports_writes(self, store: WrapperStore) -> None: assert store.supports_writes - def test_store_supports_partial_writes(self, store: WrapperStore) -> None: - assert store.supports_partial_writes - def test_store_supports_listing(self, store: WrapperStore) -> None: assert store.supports_listing diff --git a/tests/test_store/test_zip.py b/tests/test_store/test_zip.py index 24b25ed315..744ee82945 100644 --- a/tests/test_store/test_zip.py +++ b/tests/test_store/test_zip.py @@ -72,9 +72,6 @@ def test_store_repr(self, store: ZipStore) -> None: def test_store_supports_writes(self, store: ZipStore) -> None: assert store.supports_writes - def test_store_supports_partial_writes(self, store: ZipStore) -> None: - assert store.supports_partial_writes is False - def test_store_supports_listing(self, store: ZipStore) -> None: assert store.supports_listing From 314d45e372c1d6b56ceee1d7778a16c9d384c0b2 Mon Sep 17 00:00:00 2001 From: Stephan Hoyer Date: Thu, 28 Aug 2025 10:59:22 -0700 Subject: [PATCH 2/3] ruff fix --- src/zarr/abc/store.py | 1 - src/zarr/storage/_fsspec.py | 1 - src/zarr/storage/_obstore.py | 1 - src/zarr/storage/_wrapper.py | 1 - 4 files changed, 4 deletions(-) diff --git a/src/zarr/abc/store.py b/src/zarr/abc/store.py index c94ecfdd4b..e8d1329b17 100644 --- a/src/zarr/abc/store.py +++ b/src/zarr/abc/store.py @@ -12,7 +12,6 @@ from typing import Any, Self, TypeAlias from zarr.core.buffer import Buffer, BufferPrototype - from zarr.core.common import BytesLike __all__ = ["ByteGetter", "ByteSetter", "Store", "set_or_delete"] diff --git a/src/zarr/storage/_fsspec.py b/src/zarr/storage/_fsspec.py index cb6b4f55ed..3e85a1891c 100644 --- a/src/zarr/storage/_fsspec.py +++ b/src/zarr/storage/_fsspec.py @@ -26,7 +26,6 @@ from fsspec.mapping import FSMap from zarr.core.buffer import BufferPrototype - from zarr.core.common import BytesLike ALLOWED_EXCEPTIONS: tuple[type[Exception], ...] = ( diff --git a/src/zarr/storage/_obstore.py b/src/zarr/storage/_obstore.py index 6bf60655c6..9bf1df8d8c 100644 --- a/src/zarr/storage/_obstore.py +++ b/src/zarr/storage/_obstore.py @@ -23,7 +23,6 @@ from obstore.store import ObjectStore as _UpstreamObjectStore from zarr.core.buffer import Buffer, BufferPrototype - from zarr.core.common import BytesLike __all__ = ["ObjectStore"] diff --git a/src/zarr/storage/_wrapper.py b/src/zarr/storage/_wrapper.py index dccdd35f8d..ba300a4085 100644 --- a/src/zarr/storage/_wrapper.py +++ b/src/zarr/storage/_wrapper.py @@ -9,7 +9,6 @@ from zarr.abc.store import ByteRequest from zarr.core.buffer import Buffer, BufferPrototype - from zarr.core.common import BytesLike from zarr.abc.store import Store From 6cd571ff8ad19d60cb554f1a1317974b399786e4 Mon Sep 17 00:00:00 2001 From: Stephan Hoyer Date: Thu, 28 Aug 2025 11:05:01 -0700 Subject: [PATCH 3/3] include fsspec --- src/zarr/storage/_fsspec.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/zarr/storage/_fsspec.py b/src/zarr/storage/_fsspec.py index 3e85a1891c..c5afed521c 100644 --- a/src/zarr/storage/_fsspec.py +++ b/src/zarr/storage/_fsspec.py @@ -89,7 +89,6 @@ class FsspecStore(Store): allowed_exceptions supports_writes supports_deletes - supports_partial_writes supports_listing Raises @@ -113,7 +112,6 @@ class FsspecStore(Store): # based on FSSpec supports_writes: bool = True supports_deletes: bool = True - supports_partial_writes: bool = False supports_listing: bool = True fs: AsyncFileSystem