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

Add zarr.js #21

Merged
merged 4 commits into from
Feb 25, 2021
Merged

Add zarr.js #21

merged 4 commits into from
Feb 25, 2021

Conversation

manzt
Copy link
Member

@manzt manzt commented Feb 25, 2021

#19 @joshmoore @jakirkham

This PR adds a node-js based zarr writer for zarr.js. An important note is that due to the many runtime environments for JavaScript, there is no default FSStore in Zarr.js. This is because each runtime environment (e.g. node, deno, and web-browsers) provide different APIs for interfacing with the file system. Zarr.js doesn't rely on any platform specific APIs, which makes it more portable as a library, but I had to add a minimal FSStore implementation to this repo to enable writing to disk.

As a side note, I found the ergonomics of the zarr.js API for creating stores to not be very user friendly. This is probably because zarr.js use-cases are almost entirely read-only in the web-browser. It is probably ideal to add zarr.js as a reader (in a browser environment), but I'm not sure what steps to take in that direction given the pytest env.

@manzt
Copy link
Member Author

manzt commented Feb 25, 2021

Tests are passing for zarr reader but not z5py...

E       RuntimeError: [json.exception.type_error.302] type must be number, but is null

I think this has something to do with the default fill_value in zarr.js. Not sure when I'll have time to explore further.

@joshmoore
Copy link
Member

test/test_read_all.py::test_correct_read[read js zarr using z5py, blosc] PASSED [  2%]
test/test_read_all.py::test_correct_read[read js zarr using z5py, gzip] PASSED [  4%]
test/test_read_all.py::test_correct_read[read js zarr using z5py, raw] PASSED [  6%]
test/test_read_all.py::test_correct_read[read js zarr using z5py, zlib] PASSED [  8%]
test/test_read_all.py::test_correct_read[read js zarr using zarr, blosc] PASSED [ 10%]
test/test_read_all.py::test_correct_read[read js zarr using zarr, gzip] PASSED [ 12%]
test/test_read_all.py::test_correct_read[read js zarr using zarr, raw] PASSED [ 14%]
test/test_read_all.py::test_correct_read[read js zarr using zarr, zlib] PASSED [ 16%]

Very good stuff! I notice things aren't really set up for reading with non-Python languages. Happy to get this merged while some thought is put into that.

@constantinpape
Copy link
Collaborator

@manzt looks great! I think this is good to be merged, any objections?

@joshmoore

Very good stuff! I notice things aren't really set up for reading with non-Python languages.

Indeed, we should add read tests for other languages.

@manzt
Copy link
Member Author

manzt commented Feb 25, 2021

No objections on my end!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants