-
Notifications
You must be signed in to change notification settings - Fork 63
Define parser-specific Store protocols using obspec primitives #859
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,27 @@ | ||
| from collections.abc import Iterable | ||
| from typing import Protocol | ||
|
|
||
| import ujson | ||
| from obspec import Get | ||
| from obspec_utils.registry import ObjectStoreRegistry | ||
|
|
||
| from virtualizarr.manifests import ManifestStore | ||
| from virtualizarr.parsers.kerchunk.translator import manifestgroup_from_kerchunk_refs | ||
|
|
||
|
|
||
| class KerchunkJSONParser: | ||
| """Parser for Kerchunk JSON reference files.""" | ||
|
|
||
| class Store(Get, Protocol): | ||
| """ | ||
| Store protocol required by KerchunkJSONParser. | ||
|
|
||
| KerchunkJSONParser only needs to fetch the entire JSON file, so it | ||
| requires only the Get protocol from obspec. | ||
| """ | ||
|
|
||
| pass | ||
|
|
||
| def __init__( | ||
| self, | ||
| group: str | None = None, | ||
|
|
@@ -35,7 +49,7 @@ def __init__( | |
| def __call__( | ||
| self, | ||
| url: str, | ||
| registry: ObjectStoreRegistry, | ||
| registry: ObjectStoreRegistry["KerchunkJSONParser.Store"], | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you put this at the top of the file, you can drop the quotes: from __future__ import annotationsThat's why you didn't need the quotes in |
||
| ) -> ManifestStore: | ||
| """ | ||
| Parse the metadata and byte offsets from a given Kerchunk JSON to produce a | ||
|
|
@@ -46,7 +60,10 @@ def __call__( | |
| url | ||
| The URL of the input Kerchunk JSON (e.g., "s3://bucket/kerchunk.json"). | ||
| registry | ||
| An [ObjectStoreRegistry][obspec_utils.registry.ObjectStoreRegistry] for resolving urls and reading data. | ||
| An [ObjectStoreRegistry][obspec_utils.registry.ObjectStoreRegistry] for | ||
| resolving urls and reading data. The registry must contain stores that | ||
| implement the [KerchunkJSONParser.Store][virtualizarr.parsers.kerchunk.json.KerchunkJSONParser.Store] | ||
| protocol (Get operation). | ||
|
|
||
| Returns | ||
| ------- | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,14 +1,20 @@ | ||
| from __future__ import annotations | ||
|
|
||
| from typing import Callable, Protocol, runtime_checkable | ||
| from typing import Any, Callable, Protocol, runtime_checkable | ||
|
|
||
| from obspec_utils.obspec import ReadableFile, ReadableStore | ||
| from obspec_utils.protocols import ReadableFile | ||
| from obspec_utils.registry import ObjectStoreRegistry | ||
|
|
||
| from virtualizarr.manifests import ManifestStore | ||
|
|
||
| # Type alias for reader factories | ||
| ReaderFactory = Callable[[ReadableStore, str], ReadableFile] | ||
| # Store type is Any because different readers have different protocol requirements: | ||
| # - BufferedStoreReader needs Get + GetRange | ||
| # - EagerStoreReader needs Get + GetRanges + Head | ||
| # - ParallelStoreReader needs Get + GetRanges + Head | ||
| # Each reader's __init__ declares its specific Store protocol for static type checking. | ||
| # At runtime, missing methods will raise AttributeError when called. | ||
| ReaderFactory = Callable[[Any, str], ReadableFile] | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider allowing for more precise typing: See related comment in |
||
|
|
||
|
|
||
| @runtime_checkable | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Put this at the top of the file in order to drop the quotes: