-
Notifications
You must be signed in to change notification settings - Fork 52
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
Add zarr v2 endpoints to Tiled #774
base: main
Are you sure you want to change the base?
Conversation
FIX: Recursion error when pickling with dill TST: Initial tests for zarr endpoints Clean, refactor, and lint TST: tests for arrays and tables TST: tests for arrays and tables ENH: restructure demo examples ENH: (partial) support for StructDtype TST: fix tests ENH: support for datetime types
pyproject.toml
Outdated
@@ -44,6 +44,7 @@ tiled = "tiled.commandline.main:main" | |||
|
|||
# This is the union of all optional dependencies. | |||
all = [ | |||
"aiohttp", |
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.
I cannot find where this is used. Maybe this was needed in some transient state of the PR but no longer is needed.
$ git grep aiohttp
pyproject.toml: "aiohttp",
pyproject.toml: "aiohttp",
pyproject.toml: "aiohttp",
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.
Moreover, since we use starlette
for the server and httpx
for the client, it would be somewhat odd and redundant to use aiohttp
as well.
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.
It is used by fsspec.implementations.http.HTTPFileSystem
, which is needed to connect to a tiled server that requires authentication. I had the same thought yesterday, that this was something I used before but no longer need, but unfortunately it's not the case. We don't need it in all requirements though (only for testing), which I have fixed now.
arr = zarr.open(fs.get_mapper(url), mode="r") | ||
actual = arr[...] | ||
expected = df[col] | ||
assert numpy.array_equal(actual, expected) |
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.
Attempting to write raises a helpful error message:
ReadOnlyError: object is read-only
This behavior should be tested.
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.
Added a couple of test cases here. Just to note, those errors are raised by fsspec
and zarr
client objects, even before the request reaches Tiled.
This PR exposes Tiled data as a zarr collection on a set of new api endpoints,
/zarr/v2/...
. This allows one to use zarr clients directly with Tiled, as if it was an external filesystem accessed throughfsspec
.Assuming a demo Tiled server is running on
127.0.0.1:8000
(e.g. started withtiled serve demo
), one can read its contents into zarr by first specifying a file system mapper and then passing it to zarr:The resulting object is a zarr.Group, which represents the root of the Tiled catalog tree and supports (most) of the usual operations on zarr groups:
The native tiled datastructures are mapped to zarr as follows:
Addresses the Issue #562.
Checklist