Skip to content

Commit

Permalink
rename RemoteStore -> FsspecStore (#2557)
Browse files Browse the repository at this point in the history
* rename RemoteStore -> FsspecStore

* release note

* fix store type

* fixup doc build
  • Loading branch information
jhamman authored Dec 15, 2024
1 parent 122760f commit ab1a7b3
Show file tree
Hide file tree
Showing 7 changed files with 42 additions and 40 deletions.
2 changes: 2 additions & 0 deletions docs/release.rst
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ Dependency Changes
fsspec and any relevant implementations (e.g. s3fs) before using the ``RemoteStore``.
By :user:`Joe Hamman <jhamman>` :issue:`2391`.

* ``RemoteStore`` was renamed to ``FsspecStore``.
By :user:`Joe Hamman <jhamman>` :issue:`2557`.

.. release_3.0.0-alpha:
Expand Down
4 changes: 2 additions & 2 deletions src/zarr/storage/__init__.py
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
from zarr.storage.common import StoreLike, StorePath, make_store_path
from zarr.storage.fsspec import FsspecStore
from zarr.storage.local import LocalStore
from zarr.storage.logging import LoggingStore
from zarr.storage.memory import MemoryStore
from zarr.storage.remote import RemoteStore
from zarr.storage.wrapper import WrapperStore
from zarr.storage.zip import ZipStore

__all__ = [
"FsspecStore",
"LocalStore",
"LoggingStore",
"MemoryStore",
"RemoteStore",
"StoreLike",
"StorePath",
"WrapperStore",
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 @@ -281,7 +281,7 @@ async def make_store_path(
TypeError
If the StoreLike object is not one of the supported types.
"""
from zarr.storage.remote import RemoteStore # circular import
from zarr.storage.fsspec import FsspecStore # circular import

used_storage_options = False
path_normalized = normalize_path(path)
Expand All @@ -302,7 +302,7 @@ async def make_store_path(

if _is_fsspec_uri(store_like):
used_storage_options = True
store = RemoteStore.from_url(
store = FsspecStore.from_url(
store_like, storage_options=storage_options, read_only=_read_only
)
else:
Expand Down
22 changes: 11 additions & 11 deletions src/zarr/storage/remote.py → src/zarr/storage/fsspec.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
)


class RemoteStore(Store):
class FsspecStore(Store):
"""
A remote Store based on FSSpec
Expand Down Expand Up @@ -61,8 +61,8 @@ class RemoteStore(Store):
See Also
--------
RemoteStore.from_upath
RemoteStore.from_url
FsspecStore.from_upath
FsspecStore.from_url
"""

# based on FSSpec
Expand Down Expand Up @@ -96,17 +96,17 @@ def __init__(
if "://" in path and not path.startswith("http"):
# `not path.startswith("http")` is a special case for the http filesystem (¯\_(ツ)_/¯)
scheme, _ = path.split("://", maxsplit=1)
raise ValueError(f"path argument to RemoteStore must not include scheme ({scheme}://)")
raise ValueError(f"path argument to FsspecStore must not include scheme ({scheme}://)")

@classmethod
def from_upath(
cls,
upath: Any,
read_only: bool = False,
allowed_exceptions: tuple[type[Exception], ...] = ALLOWED_EXCEPTIONS,
) -> RemoteStore:
) -> FsspecStore:
"""
Create a RemoteStore from an upath object.
Create a FsspecStore from an upath object.
Parameters
----------
Expand All @@ -120,7 +120,7 @@ def from_upath(
Returns
-------
RemoteStore
FsspecStore
"""
return cls(
fs=upath.fs,
Expand All @@ -136,9 +136,9 @@ def from_url(
storage_options: dict[str, Any] | None = None,
read_only: bool = False,
allowed_exceptions: tuple[type[Exception], ...] = ALLOWED_EXCEPTIONS,
) -> RemoteStore:
) -> FsspecStore:
"""
Create a RemoteStore from a URL.
Create a FsspecStore from a URL.
Parameters
----------
Expand All @@ -154,7 +154,7 @@ def from_url(
Returns
-------
RemoteStore
FsspecStore
"""
try:
from fsspec import url_to_fs
Expand Down Expand Up @@ -185,7 +185,7 @@ async def clear(self) -> None:
pass

def __repr__(self) -> str:
return f"<RemoteStore({type(self.fs).__name__}, {self.path})>"
return f"<FsspecStore({type(self.fs).__name__}, {self.path})>"

def __eq__(self, other: object) -> bool:
return (
Expand Down
16 changes: 8 additions & 8 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
from zarr.abc.store import Store
from zarr.core.sync import sync
from zarr.storage import LocalStore, MemoryStore, StorePath, ZipStore
from zarr.storage.remote import RemoteStore
from zarr.storage.fsspec import FsspecStore

if TYPE_CHECKING:
from collections.abc import Generator
Expand All @@ -25,14 +25,14 @@


async def parse_store(
store: Literal["local", "memory", "remote", "zip"], path: str
) -> LocalStore | MemoryStore | RemoteStore | ZipStore:
store: Literal["local", "memory", "fsspec", "zip"], path: str
) -> LocalStore | MemoryStore | FsspecStore | ZipStore:
if store == "local":
return await LocalStore.open(path)
if store == "memory":
return await MemoryStore.open()
if store == "remote":
return await RemoteStore.open(url=path)
if store == "fsspec":
return await FsspecStore.open(url=path)
if store == "zip":
return await ZipStore.open(path + "/zarr.zip", mode="w")
raise AssertionError
Expand All @@ -56,8 +56,8 @@ async def local_store(tmpdir: LEGACY_PATH) -> LocalStore:


@pytest.fixture
async def remote_store(url: str) -> RemoteStore:
return await RemoteStore.open(url)
async def remote_store(url: str) -> FsspecStore:
return await FsspecStore.open(url)


@pytest.fixture
Expand Down Expand Up @@ -87,7 +87,7 @@ def sync_store(request: pytest.FixtureRequest, tmp_path: LEGACY_PATH) -> Store:
@dataclass
class AsyncGroupRequest:
zarr_format: ZarrFormat
store: Literal["local", "remote", "memory", "zip"]
store: Literal["local", "fsspec", "memory", "zip"]
attributes: dict[str, Any] = field(default_factory=dict)


Expand Down
4 changes: 2 additions & 2 deletions tests/test_store/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@
from zarr.core.common import AccessModeLiteral
from zarr.storage._utils import normalize_path
from zarr.storage.common import StoreLike, StorePath, make_store_path
from zarr.storage.fsspec import FsspecStore
from zarr.storage.local import LocalStore
from zarr.storage.memory import MemoryStore
from zarr.storage.remote import RemoteStore


@pytest.mark.parametrize("path", [None, "", "bar"])
Expand Down Expand Up @@ -73,7 +73,7 @@ async def test_make_store_path_invalid() -> None:
async def test_make_store_path_fsspec(monkeypatch) -> None:
pytest.importorskip("fsspec")
store_path = await make_store_path("http://foo.com/bar")
assert isinstance(store_path.store, RemoteStore)
assert isinstance(store_path.store, FsspecStore)


@pytest.mark.parametrize(
Expand Down
30 changes: 15 additions & 15 deletions tests/test_store/test_remote.py → tests/test_store/test_fsspec.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import zarr.api.asynchronous
from zarr.core.buffer import Buffer, cpu, default_buffer_prototype
from zarr.core.sync import _collect_aiterator, sync
from zarr.storage import RemoteStore
from zarr.storage import FsspecStore
from zarr.testing.store import StoreTests

if TYPE_CHECKING:
Expand Down Expand Up @@ -84,7 +84,7 @@ def s3(s3_base: None) -> Generator[s3fs.S3FileSystem, None, None]:


async def test_basic() -> None:
store = RemoteStore.from_url(
store = FsspecStore.from_url(
f"s3://{test_bucket_name}/foo/spam/",
storage_options={"endpoint_url": endpoint_url, "anon": False},
)
Expand All @@ -102,8 +102,8 @@ async def test_basic() -> None:
assert out[0].to_bytes() == data[1:]


class TestRemoteStoreS3(StoreTests[RemoteStore, cpu.Buffer]):
store_cls = RemoteStore
class TestFsspecStoreS3(StoreTests[FsspecStore, cpu.Buffer]):
store_cls = FsspecStore
buffer_cls = cpu.Buffer

@pytest.fixture
Expand All @@ -114,36 +114,36 @@ def store_kwargs(self, request) -> dict[str, str | bool]:
return {"fs": fs, "path": path}

@pytest.fixture
def store(self, store_kwargs: dict[str, str | bool]) -> RemoteStore:
def store(self, store_kwargs: dict[str, str | bool]) -> FsspecStore:
return self.store_cls(**store_kwargs)

async def get(self, store: RemoteStore, key: str) -> Buffer:
async def get(self, store: FsspecStore, key: str) -> Buffer:
# make a new, synchronous instance of the filesystem because this test is run in sync code
new_fs = fsspec.filesystem(
"s3", endpoint_url=store.fs.endpoint_url, anon=store.fs.anon, asynchronous=False
)
return self.buffer_cls.from_bytes(new_fs.cat(f"{store.path}/{key}"))

async def set(self, store: RemoteStore, key: str, value: Buffer) -> None:
async def set(self, store: FsspecStore, key: str, value: Buffer) -> None:
# make a new, synchronous instance of the filesystem because this test is run in sync code
new_fs = fsspec.filesystem(
"s3", endpoint_url=store.fs.endpoint_url, anon=store.fs.anon, asynchronous=False
)
new_fs.write_bytes(f"{store.path}/{key}", value.to_bytes())

def test_store_repr(self, store: RemoteStore) -> None:
assert str(store) == "<RemoteStore(S3FileSystem, test)>"
def test_store_repr(self, store: FsspecStore) -> None:
assert str(store) == "<FsspecStore(S3FileSystem, test)>"

def test_store_supports_writes(self, store: RemoteStore) -> None:
def test_store_supports_writes(self, store: FsspecStore) -> None:
assert store.supports_writes

def test_store_supports_partial_writes(self, store: RemoteStore) -> None:
def test_store_supports_partial_writes(self, store: FsspecStore) -> None:
assert not store.supports_partial_writes

def test_store_supports_listing(self, store: RemoteStore) -> None:
def test_store_supports_listing(self, store: FsspecStore) -> None:
assert store.supports_listing

async def test_remote_store_from_uri(self, store: RemoteStore):
async def test_fsspec_store_from_uri(self, store: FsspecStore) -> None:
storage_options = {
"endpoint_url": endpoint_url,
"anon": False,
Expand Down Expand Up @@ -188,7 +188,7 @@ def test_from_upath(self) -> None:
anon=False,
asynchronous=True,
)
result = RemoteStore.from_upath(path)
result = FsspecStore.from_upath(path)
assert result.fs.endpoint_url == endpoint_url
assert result.fs.asynchronous
assert result.path == f"{test_bucket_name}/foo/bar"
Expand All @@ -197,7 +197,7 @@ def test_init_raises_if_path_has_scheme(self, store_kwargs) -> None:
# regression test for https://github.com/zarr-developers/zarr-python/issues/2342
store_kwargs["path"] = "s3://" + store_kwargs["path"]
with pytest.raises(
ValueError, match="path argument to RemoteStore must not include scheme .*"
ValueError, match="path argument to FsspecStore must not include scheme .*"
):
self.store_cls(**store_kwargs)

Expand Down

0 comments on commit ab1a7b3

Please sign in to comment.