{{ message }}
Extensible codecs for V3#1588
Merged
Merged
Conversation
No code changes to Zarrita were made.
jhamman
approved these changes
Dec 6, 2023
jhamman
left a comment
jhamman
left a comment
Member
There was a problem hiding this comment.
I mostly reviewed the entrypoints stuff since the rest seemed to be reorganization / porting from zarrita. Looks really nice!
| @@ -59,24 +53,8 @@ def _structure_chunk_key_encoding_metadata(d: Dict[str, Any], _t) -> ChunkKeyEnc | |||
| ) | |||
|
|
|||
| def _structure_codec_metadata(d: Dict[str, Any], _t=None) -> CodecMetadata: | |||
Member
There was a problem hiding this comment.
not for now but it would be nice if the codec metadata was a TypedDict so we could expect for keys like "name".
Member
Author
There was a problem hiding this comment.
name is really the only shared attribute among all codecs. Even configuration is optional. I don't think there'd be much value in a TypedDict. This piece of code is in the middle of the deserialization; lots of untyped magic happening here anyways :-)
Co-authored-by: Joe Hamman <jhamman1@gmail.com>
normanrz
commented
Dec 6, 2023
Comment on lines
+19
to
+36
Member
Author
This was referenced Dec 7, 2023
jhamman
added a commit
to jhamman/zarr-python
that referenced
this pull request
Jan 24, 2024
* Pull Zarrita into Zarr-Python @ 78274781ad64aef95772eb4b083f7ea9b7d03d06 No code changes to Zarrita were made. * apply zarr lint rules * zarrita -> v3 * v3/abc [wip] * use abcs plus implementation notes * working on making codecs extensible * adds index_location * adds support for codec entry points * adds tests from zarrita * fixes types * Apply suggestions from code review Co-authored-by: Joe Hamman <jhamman1@gmail.com> * remove test codec from pyproject.toml --------- Co-authored-by: Joseph Hamman <joe@earthmover.io> Co-authored-by: Joe Hamman <jhamman1@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Makes codecs extensible in the v3 branch by adding a codec registry.