-
Notifications
You must be signed in to change notification settings - Fork 24
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
Implement ZstdZarrCompressor #149
base: master
Are you sure you want to change the base?
Conversation
Pull Request Test Coverage Report for Build 9654302116Details
💛 - Coveralls |
Pull Request Test Coverage Report for Build 9655786902Details
💛 - Coveralls |
Pull Request Test Coverage Report for Build 9656097799Details
💛 - Coveralls |
Pull Request Test Coverage Report for Build 9657292182Details
💛 - Coveralls |
An alternative to the string lookup for the compressor, would be to just pass in |
struct ZstdZarrCompressor <: Zarr.Compressor | ||
compressor::CodecZstd.ZstdCompressor | ||
decompressor::CodecZstd.ZstdDecompressor | ||
end |
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.
This doesn't support multithreaded use IIUC. I think this should be like
Lines 129 to 131 in f436713
struct ZlibCompressor <: Compressor | |
clevel::Int | |
end |
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 initially wrote it like this, but then I was thinking about all the other potential parameters, even if they do not need to be serialized. I think what we should implement is the ability to copy a compessor.
Frankly, I'm somewhat confused about why one actually needs to serialize the compression level into the array metadata. You do not need that information to decompress the data.
This implements ZstdZarrCompressor which wraps around CodecZstd as a package extension.
Part of the complication of using package extensions is getting a reference to new types defined in the extension. I created a mechanism by which you could specify the compressor as a string, which would then lookup the type from a dictionary.
I'm also wondering if there might be a general way to wrap TranscodingStreams codecs into Zarr compressors.