diff --git a/CHANGELOG.md b/CHANGELOG.md index 2128b4e..37be3b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,3 @@ # Changelog -For more information, see the [changelog](https://redsun-acquisition.github.io/sunflare/main/changelog/). +For more information, see the [changelog](https://redsun-acquisition.github.io/sunflare/reference/changelog/). diff --git a/docs/explanations/architecture/storage.md b/docs/explanations/architecture/storage.md index 8b66f2c..c2a8b8a 100644 --- a/docs/explanations/architecture/storage.md +++ b/docs/explanations/architecture/storage.md @@ -101,7 +101,7 @@ strategies that can be composed with StaticFilenameProvider("scan001"), base_uri="file:///data", ) - # Produces: file:///data/scan001 (array key = device name) + # Produces a store at /data/scan001 (array key = device name) ``` === "UUID filename" @@ -114,7 +114,7 @@ strategies that can be composed with UUIDFilenameProvider(), base_uri="file:///data", ) - # Produces: file:///data/3f2504e0-... (array key = device name) + # Produces a store at /data/3f2504e0-... (array key = device name) ``` === "Auto-increment filename" @@ -129,9 +129,6 @@ strategies that can be composed with ) ``` -[`PathInfo`][sunflare.storage.PathInfo] uses URIs rather than filesystem paths, -so the same provider interface works for both local (`file://`) and remote -(`s3://`) backends. --- @@ -152,9 +149,8 @@ class StorageProxy(Protocol): def collect_stream_docs(self, name, indices_written) -> Iterator[StreamAsset]: ... ``` -[`Writer`][sunflare.storage.Writer] satisfies this protocol structurally. Future -remote proxy objects will too, so device code is identical regardless of whether -storage is local or remote. +[`Writer`][sunflare.storage.Writer] satisfies this protocol structurally, so device +code remains independent of the concrete backend. !!! tip When testing devices in isolation, pass a `MagicMock(spec=StorageProxy)` as diff --git a/docs/explanations/index.md b/docs/explanations/index.md index 875a56b..408419e 100644 --- a/docs/explanations/index.md +++ b/docs/explanations/index.md @@ -7,3 +7,4 @@ - [Devices](architecture/devices.md) - [Presenters](architecture/presenters.md) - [Virtual container](architecture/virtual.md) +- [Storage](architecture/storage.md) diff --git a/docs/reference/api.md b/docs/reference/api.md index 4d90a83..e6540e9 100644 --- a/docs/reference/api.md +++ b/docs/reference/api.md @@ -1,31 +1,37 @@ # API reference -::: sunflare.storage - options: - summary: true - ::: sunflare.device options: summary: true +--- + ::: sunflare.presenter options: summary: true +--- + ::: sunflare.virtual options: summary: true +--- + ::: sunflare.view options: summary: true +--- + ::: sunflare.log options: members: - Loggable summary: true +--- + ::: sunflare.engine options: members: @@ -33,3 +39,9 @@ - RunEngine - RunEngineResult summary: true + +--- + +::: sunflare.storage + options: + summary: true diff --git a/src/sunflare/storage/__init__.py b/src/sunflare/storage/__init__.py index 0effe7e..beb2939 100644 --- a/src/sunflare/storage/__init__.py +++ b/src/sunflare/storage/__init__.py @@ -22,6 +22,7 @@ - [`StaticPathProvider`][sunflare.storage.StaticPathProvider] — concrete path provider - [`StorageProxy`][sunflare.storage.StorageProxy] — protocol implemented by all storage backends - [`StorageDescriptor`][sunflare.storage.StorageDescriptor] — descriptor for declaring `storage` on a device +- [`HasStorage`][sunflare.storage.HasStorage] — protocol for devices that have opted in to storage Concrete backend classes (e.g. `ZarrWriter`) are internal implementation details and are not exported from this package. @@ -51,7 +52,11 @@ class MyDetector(Device): StaticPathProvider, UUIDFilenameProvider, ) -from sunflare.storage._proxy import StorageDescriptor, StorageProxy +from sunflare.storage._proxy import ( + HasStorage, + StorageDescriptor, + StorageProxy, +) __all__ = [ # base @@ -69,4 +74,5 @@ class MyDetector(Device): # proxy / descriptor "StorageProxy", "StorageDescriptor", + "HasStorage", ] diff --git a/src/sunflare/storage/_path.py b/src/sunflare/storage/_path.py index ccd664a..b1decc3 100644 --- a/src/sunflare/storage/_path.py +++ b/src/sunflare/storage/_path.py @@ -4,7 +4,7 @@ # ophyd-async is licensed under the BSD 3-Clause License. # No source code from ophyd-async has been copied; the PathProvider / FilenameProvider # composable pattern was studied and independently re-implemented for sunflare, -# with URI-based paths to support non-POSIX backends such as S3. +# with URI-based paths for storage location flexibility. """Path and filename providers for storage backends.""" @@ -22,9 +22,8 @@ class PathInfo: Attributes ---------- store_uri : str - URI of the store root. For local Zarr this is a `file://` URI; - for remote storage it may be `s3://` or similar. - Example: `"file:///data/scan001.zarr"`. + URI of the store root. For local Zarr this is a ``file://`` URI. + Example: ``"file:///data/scan001.zarr"``. array_key : str Key (array name) within the store for this device's data. Defaults to the device name. @@ -68,12 +67,7 @@ def __call__(self, device_name: str | None = None) -> str: @runtime_checkable class PathProvider(Protocol): - """Callable that produces [`PathInfo`][sunflare.storage.PathInfo] for a device. - - Implementations are **picklable** — they carry no open file handles - or mutable process-local state, so they can be safely forwarded to - subprocess or remote workers. - """ + """Callable that produces [`PathInfo`][sunflare.storage.PathInfo] for a device.""" def __call__(self, device_name: str | None = None) -> PathInfo: """Return path information for the given device. diff --git a/src/sunflare/storage/_proxy.py b/src/sunflare/storage/_proxy.py index fc2dd45..b202097 100644 --- a/src/sunflare/storage/_proxy.py +++ b/src/sunflare/storage/_proxy.py @@ -9,7 +9,14 @@ from __future__ import annotations -from typing import TYPE_CHECKING, Any, Protocol, overload, runtime_checkable +from typing import ( + TYPE_CHECKING, + Any, + Protocol, + _ProtocolMeta, + overload, + runtime_checkable, +) if TYPE_CHECKING: from collections.abc import Iterator @@ -24,9 +31,8 @@ class StorageProxy(Protocol): """Protocol that devices use to interact with a storage backend. - Both local [`Writer`][sunflare.storage.Writer] instances and future - remote proxy objects implement this protocol, so device code is - identical regardless of where storage lives. + [`Writer`][sunflare.storage.Writer] instances implement this protocol, + so device code remains independent of the concrete backend. Devices access the backend via their `storage` attribute, which is `None` when no backend has been configured for the session. @@ -67,6 +73,23 @@ def collect_stream_docs( ... +class _HasStorageMeta(_ProtocolMeta): + """Metaclass for [HasStorage][sunflare.storage.HasStorage] that overrides `__instancecheck__`.""" + + def __instancecheck__(cls, instance: object) -> bool: + return any( + isinstance(vars(c).get("storage"), StorageDescriptor) + for c in type(instance).__mro__ + ) + + +@runtime_checkable +class HasStorage(Protocol, metaclass=_HasStorageMeta): + """Protocol for devices that have opted in to storage.""" + + storage: StorageProxy + + class StorageDescriptor: """Descriptor that manages the `storage` slot on a device. diff --git a/src/sunflare/storage/_zarr.py b/src/sunflare/storage/_zarr.py index c09f6bc..d1d13f9 100644 --- a/src/sunflare/storage/_zarr.py +++ b/src/sunflare/storage/_zarr.py @@ -9,6 +9,7 @@ from __future__ import annotations +from pathlib import Path # noqa: TC003 from typing import TYPE_CHECKING try: @@ -54,9 +55,14 @@ class ZarrWriter(Writer): Callable that returns [`PathInfo`][sunflare.storage.PathInfo] for each device. Called once per device per [`prepare`][sunflare.storage.Writer.prepare] invocation. + base_dir : Path + Filesystem directory under which all stores for this writer are + created. ``kickoff()`` ensures this directory exists before + opening the stream, so the caller does not need to ``mkdir`` it + in advance. """ - def __init__(self, name: str, path_provider: PathProvider) -> None: + def __init__(self, name: str, path_provider: PathProvider, base_dir: Path) -> None: if not _ACQUIRE_ZARR_AVAILABLE: raise ImportError( "ZarrWriter requires the 'acquire-zarr' package. " @@ -64,6 +70,7 @@ def __init__(self, name: str, path_provider: PathProvider) -> None: ) super().__init__(name) self._path_provider = path_provider + self._base_dir = base_dir self._stream_settings = StreamSettings() self._dimensions: dict[str, list[Dimension]] = {} self._array_settings: dict[str, ArraySettings] = {} @@ -136,6 +143,7 @@ def kickoff(self) -> None: """Open the Zarr stream for writing. No-op if already open.""" if self.is_open: return + self._base_dir.mkdir(parents=True, exist_ok=True) self._stream_settings.arrays = list(self._array_settings.values()) self._stream = ZarrStream(self._stream_settings) super().kickoff() diff --git a/tests/test_storage.py b/tests/test_storage.py index d18021d..fb45e25 100644 --- a/tests/test_storage.py +++ b/tests/test_storage.py @@ -2,6 +2,7 @@ from __future__ import annotations +from pathlib import Path from typing import Any from unittest.mock import MagicMock @@ -426,4 +427,51 @@ def test_import_error_without_acquire_zarr( fp = StaticFilenameProvider("scan") pp = StaticPathProvider(fp, base_uri="file:///data") with pytest.raises(ImportError, match="acquire-zarr"): - ZarrWriter("test", pp) + ZarrWriter("test", pp, Path("/data")) + + +class TestZarrWriterBaseDir: + """Tests for ZarrWriter base_dir creation at kickoff.""" + + def test_kickoff_creates_base_dir(self, tmp_path: Path) -> None: + """kickoff() creates base_dir if it does not exist yet.""" + from unittest.mock import MagicMock, patch + from sunflare.storage._zarr import ZarrWriter + from sunflare.storage import StaticFilenameProvider, StaticPathProvider + + base_dir = tmp_path / "scans" + assert not base_dir.exists() + + fp = StaticFilenameProvider("run001") + pp = StaticPathProvider(fp, base_uri=base_dir.as_uri()) + writer = ZarrWriter("test-writer", pp, base_dir) + + writer.update_source("cam", dtype=np.dtype("uint16"), shape=(64, 64)) + + with patch("sunflare.storage._zarr.ZarrStream"): + writer.prepare("cam", capacity=10) + writer.kickoff() + + assert base_dir.exists() + assert base_dir.is_dir() + + def test_kickoff_base_dir_already_exists(self, tmp_path: Path) -> None: + """kickoff() is a no-op mkdir when base_dir already exists.""" + from unittest.mock import patch + from sunflare.storage._zarr import ZarrWriter + from sunflare.storage import StaticFilenameProvider, StaticPathProvider + + base_dir = tmp_path / "scans" + base_dir.mkdir() + + fp = StaticFilenameProvider("run001") + pp = StaticPathProvider(fp, base_uri=base_dir.as_uri()) + writer = ZarrWriter("test-writer", pp, base_dir) + + writer.update_source("cam", dtype=np.dtype("uint16"), shape=(64, 64)) + + with patch("sunflare.storage._zarr.ZarrStream"): + writer.prepare("cam", capacity=10) + writer.kickoff() # must not raise + + assert base_dir.exists()