Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

more extensive api x store testing #2447

Open
wants to merge 46 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
e309415
add filenotfounderror to tupe of handled errors
d-v-b Oct 30, 2024
0045351
revive remote store test fixture, and parameterize api tests with all…
d-v-b Oct 30, 2024
d670b30
properly construct remotestore instance in test fixture
d-v-b Oct 30, 2024
f6d35fb
add filenotfounderror to tuple of handled errors
d-v-b Oct 30, 2024
311efaf
propagate mode kwarg into open_group
d-v-b Oct 30, 2024
73911b7
propagate mode kwarg into open_group
d-v-b Oct 30, 2024
9917343
xfail zipstore
d-v-b Oct 30, 2024
90df740
revert removal of type ignores
d-v-b Oct 30, 2024
3c3099a
speculative refactor of open func
d-v-b Oct 30, 2024
0219425
refactor store test fixture
d-v-b Oct 30, 2024
bd5eb13
send s3fs fixture to remotestore fixture
d-v-b Oct 31, 2024
1c6044b
remove unused method from group and corresponding test
d-v-b Oct 31, 2024
4c5f06e
explicitly depend on moto server
d-v-b Oct 31, 2024
420ca07
explicitly depend on moto server, also in test deps
d-v-b Oct 31, 2024
bac994d
ignore mypy errors properly
d-v-b Oct 31, 2024
426e1cb
fixup
d-v-b Oct 31, 2024
0013d22
annotate store variables as Store
d-v-b Oct 31, 2024
fb52a7c
add moto[server] to min_deps
d-v-b Oct 31, 2024
53b9faf
Merge branch 'main' into fix/open-v2-array-remotestore
d-v-b Nov 5, 2024
2ab4dc3
Merge branch 'main' of github.com:zarr-developers/zarr-python into fi…
d-v-b Nov 13, 2024
9ec0d01
revert store fixture name convention
d-v-b Nov 15, 2024
7bc982e
remove unused import
d-v-b Nov 15, 2024
d776ead
remove mode arg for remotestore
d-v-b Nov 15, 2024
191c012
fixup
d-v-b Nov 15, 2024
da03f87
Merge branch 'main' of github.com:zarr-developers/zarr-python into fi…
d-v-b Nov 15, 2024
9def17d
add mutability altering funcs
d-v-b Nov 15, 2024
d1c95b2
get consolidated tests to pass
d-v-b Nov 15, 2024
a04e636
fix for open_array in r+ mode
d-v-b Nov 15, 2024
e29e072
fix broken zipstore delete_dir
d-v-b Nov 15, 2024
cbf1a61
xfail zipstore in open_array test
d-v-b Nov 15, 2024
cadfef1
add missing import
d-v-b Nov 15, 2024
f085859
Merge branch 'fix/open-v2-array-remotestore' of github.com:d-v-b/zarr…
d-v-b Nov 15, 2024
c0f777d
use create instead of open_array in fill_value test
d-v-b Nov 15, 2024
94e2468
remove xfail for zipstore in test_open_with_mode_r_plus
d-v-b Nov 15, 2024
15b0211
xfail test that fails for any warning on python 3.12 or higher with r…
d-v-b Nov 15, 2024
8d3b66c
remove xfail for zip
d-v-b Nov 15, 2024
5e5376f
lint
d-v-b Nov 15, 2024
d087e9b
fix open_array for mode r+
d-v-b Nov 15, 2024
0c03d4a
Merge branch 'fix/open-r-plus' of github.com:d-v-b/zarr-python into f…
d-v-b Nov 15, 2024
9f45997
simplify group tests by removing zip and remote store cases
d-v-b Nov 15, 2024
c0d5407
simplify test_array
d-v-b Nov 15, 2024
f946499
use zarr_format fixture
d-v-b Nov 15, 2024
c5b3588
add old open_with_mode_r test
d-v-b Nov 15, 2024
c62de51
Merge branch 'main' of github.com:zarr-developers/zarr-python into fi…
d-v-b Nov 15, 2024
f95c0fb
Merge branch 'main' of github.com:zarr-developers/zarr-python into fi…
d-v-b Nov 26, 2024
0fa90ba
add _as_immutable and _as_mutable methods to stores; use those method…
d-v-b Nov 26, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ test = [
"msgpack",
"s3fs",
"pytest-asyncio",
"moto[s3]",
"moto[s3, server]",
"flask-cors",
"flask",
"requests",
Expand Down Expand Up @@ -198,7 +198,7 @@ dependencies = [
'pytest',
'pytest-cov',
'pytest-asyncio',
'moto[s3]',
'moto[s3, server]'
]

[tool.hatch.envs.upstream.env-vars]
Expand Down Expand Up @@ -231,7 +231,7 @@ dependencies = [
'pytest',
'pytest-cov',
'pytest-asyncio',
'moto[s3]',
'moto[s3, server]',
]

[tool.hatch.envs.min_deps.scripts]
Expand Down
12 changes: 12 additions & 0 deletions src/zarr/abc/store.py
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,18 @@ async def getsize_prefix(self, prefix: str) -> int:
sizes = await concurrent_map(keys, self.getsize, limit=limit)
return sum(sizes)

def _as_immutable(self: Self) -> Self:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in icechunk we have these as set_read_only and set_writeable which I mildly prefer. Simple words are nice.

"""
Return a mutable copy of the store.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like perhaps the words "mutable" and "immutable" should be swapped between this method and the _as_mutable method, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops, good catch!

"""
raise NotImplementedError

def _as_mutable(self: Self) -> Self:
"""
Return an immutable (read-only) copy of the store.
"""
raise NotImplementedError


@runtime_checkable
class ByteGetter(Protocol):
Expand Down
4 changes: 1 addition & 3 deletions src/zarr/api/asynchronous.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@
"ones",
"ones_like",
"open",
"open_array",
"open_consolidated",
"open_group",
"open_like",
Expand Down Expand Up @@ -301,7 +300,7 @@ async def open(
store_path = await make_store_path(store, mode=mode, path=path, storage_options=storage_options)

# TODO: the mode check below seems wrong!
if "shape" not in kwargs and mode in {"a", "r", "r+"}:
if "shape" not in kwargs and mode in _READ_MODES:
try:
metadata_dict = await get_array_metadata(store_path, zarr_format=zarr_format)
# TODO: remove this cast when we fix typing for array metadata dicts
Expand Down Expand Up @@ -1093,7 +1092,6 @@ async def open_array(
store_path = await make_store_path(store, path=path, mode=mode, storage_options=storage_options)

zarr_format = _handle_zarr_version_or_format(zarr_version=zarr_version, zarr_format=zarr_format)

try:
return await AsyncArray.open(store_path, zarr_format=zarr_format)
except FileNotFoundError:
Expand Down
22 changes: 0 additions & 22 deletions src/zarr/core/group.py
Original file line number Diff line number Diff line change
Expand Up @@ -804,7 +804,6 @@ async def _save_metadata(self, ensure_parents: bool = False) -> None:
).items()
]
)

await asyncio.gather(*awaitables)

@property
Expand Down Expand Up @@ -1844,27 +1843,6 @@ def __setitem__(self, key: str, value: Any) -> None:
def __repr__(self) -> str:
return f"<Group {self.store_path}>"

async def update_attributes_async(self, new_attributes: dict[str, Any]) -> Group:
"""Update the attributes of this group.

Example
-------
>>> import zarr
>>> group = zarr.group()
>>> await group.update_attributes_async({"foo": "bar"})
>>> group.attrs.asdict()
{'foo': 'bar'}
"""
new_metadata = replace(self.metadata, attributes=new_attributes)

# Write new metadata
to_save = new_metadata.to_buffer_dict(default_buffer_prototype())
awaitables = [set_or_delete(self.store_path / key, value) for key, value in to_save.items()]
await asyncio.gather(*awaitables)

async_group = replace(self._async_group, metadata=new_metadata)
return replace(self, _async_group=async_group)

@property
def store_path(self) -> StorePath:
"""Path-like interface for the Store."""
Expand Down
4 changes: 2 additions & 2 deletions src/zarr/storage/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@
from zarr.storage.local import LocalStore
from zarr.storage.memory import MemoryStore

# from zarr.store.remote import RemoteStore

if TYPE_CHECKING:
from zarr.core.buffer import BufferPrototype

Expand Down Expand Up @@ -72,6 +70,8 @@ async def open(
"""

await store._ensure_open()
if mode == "r" and not store.read_only:
store = store._as_immutable()
self = cls(store, path)

# fastpath if mode is None
Expand Down
8 changes: 7 additions & 1 deletion src/zarr/storage/local.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import os
import shutil
from pathlib import Path
from typing import TYPE_CHECKING
from typing import TYPE_CHECKING, Self

from zarr.abc.store import ByteRangeRequest, Store
from zarr.core.buffer import Buffer
Expand Down Expand Up @@ -229,3 +229,9 @@ async def list_dir(self, prefix: str) -> AsyncIterator[str]:

async def getsize(self, key: str) -> int:
return os.path.getsize(self.root / key)

def _as_immutable(self: Self) -> Self:
return type(self)(self.root, read_only=True)

def _as_mutable(self: Self) -> Self:
return type(self)(self.root, read_only=False)
14 changes: 13 additions & 1 deletion src/zarr/storage/logging.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import time
from collections import defaultdict
from contextlib import contextmanager
from typing import TYPE_CHECKING, Any
from typing import TYPE_CHECKING, Any, Self

from zarr.abc.store import ByteRangeRequest, Store

Expand Down Expand Up @@ -233,3 +233,15 @@ async def getsize(self, key: str) -> int:
async def getsize_prefix(self, prefix: str) -> int:
with self.log(prefix):
return await self._store.getsize_prefix(prefix)

def _as_immutable(self: Self) -> Self:
return type(self)(
store=self._store._as_immutable(),
log_level=self.log_level,
log_handler=self.log_handler,
)

def _as_mutable(self: Self) -> Self:
return type(self)(
store=self._store._as_mutable(), log_level=self.log_level, log_handler=self.log_handler
)
12 changes: 12 additions & 0 deletions src/zarr/storage/memory.py
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,12 @@ async def list_dir(self, prefix: str) -> AsyncIterator[str]:
for key in keys_unique:
yield key

def _as_immutable(self: Self) -> Self:
return type(self)(self._store_dict, read_only=True)

def _as_mutable(self: Self) -> Self:
return type(self)(self._store_dict, read_only=False)


class GpuMemoryStore(MemoryStore):
"""A GPU only memory store that stores every chunk in GPU memory irrespective
Expand Down Expand Up @@ -236,3 +242,9 @@ async def set(self, key: str, value: Buffer, byte_range: tuple[int, int] | None
# Convert to gpu.Buffer
gpu_value = value if isinstance(value, gpu.Buffer) else gpu.Buffer.from_buffer(value)
await super().set(key, gpu_value, byte_range=byte_range)

def _as_immutable(self: Self) -> Self:
return type(self)(self._store_dict, read_only=True)

def _as_mutable(self: Self) -> Self:
return type(self)(self._store_dict, read_only=False)
12 changes: 11 additions & 1 deletion src/zarr/storage/remote.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from __future__ import annotations

import warnings
from typing import TYPE_CHECKING, Any
from typing import TYPE_CHECKING, Any, Self

from zarr.abc.store import ByteRangeRequest, Store
from zarr.storage.common import _dereference_path
Expand Down Expand Up @@ -338,3 +338,13 @@ async def getsize(self, key: str) -> int:
else:
# fsspec doesn't have typing. We'll need to assume or verify this is true
return int(size)

def _as_immutable(self: Self) -> Self:
return type(self)(
self.fs, read_only=True, path=self.path, allowed_exceptions=self.allowed_exceptions
)

def _as_mutable(self: Self) -> Self:
return type(self)(
self.fs, read_only=False, path=self.path, allowed_exceptions=self.allowed_exceptions
)
26 changes: 25 additions & 1 deletion src/zarr/storage/zip.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import time
import zipfile
from pathlib import Path
from typing import TYPE_CHECKING, Any, Literal
from typing import TYPE_CHECKING, Any, Literal, Self

from zarr.abc.store import ByteRangeRequest, Store
from zarr.core.buffer import Buffer, BufferPrototype
Expand Down Expand Up @@ -269,3 +269,27 @@ async def list_dir(self, prefix: str) -> AsyncIterator[str]:
if k not in seen:
seen.add(k)
yield k

def _as_immutable(self: Self) -> Self:
self.close()
new_store = type(self)(
self.path,
read_only=True,
mode="r",
compression=self.compression,
allowZip64=self.allowZip64,
)
new_store._sync_open()
return new_store

def _as_mutable(self: Self) -> Self:
self.close()
new_store = type(self)(
self.path,
read_only=False,
mode="a",
compression=self.compression,
allowZip64=self.allowZip64,
)
new_store._sync_open()
return new_store
Loading
Loading