-
-
Notifications
You must be signed in to change notification settings - Fork 365
Deprecate (Async)Array.create #2607
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
Conversation
src/zarr/core/array.py
Outdated
| shards: ShardsParam | None = None, | ||
| filters: FiltersParam | None = "auto", | ||
| compressors: CompressorsParam = "auto", | ||
| array_bytes_codec: ArrayBytesCodecParam = "auto", |
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 think this parameter is important, but I think we can do better with the name to express a bit more what it does, and detach the name from the type (which might change). Unfortunately the only ideas I have are array_serializer chunk_serializer but I'm sure there are better options.
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.
What about just codec?
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.
to me, codec is a category that includes ArrayBytesCodec and BytesBytesCodec; I think the ideal name would somehow convey that this entity is tasked with unpacking (or packing?) an array into bytes. chunk_<verb>+er seems like a good template, but I'm not sure what verbs fit well here. 🤔 chunk_serializer, chunk_packer, chunk_streamer... chunk_serializer seems the best to me but it's far from great.
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 think chunk_serializer is fine.
Maybe even just serializer. That would be more in line with filters and compressors.
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.
+1 to dropping chunk_ as long as we don't think people will be confused about the scope
…r-python into feat/no-array-create
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 looks great! But I do think we should try to improve the name of the array_bytes_codec kwarg. see #2607 (comment)
Co-authored-by: Joe Hamman <[email protected]>
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.
lgtm, thanks @normanrz
|
Merged into #2463. |
Builds upon #2463
AsyncArray.createandArray.createtoArray._createincluding deprecation warnings in favor ofzarr.create_array.AsyncArray._createandArray._createwhich now contain the code of the unprefixed methods. We should not use them for new code anymore and remove once other APIs have been deprecated and removed.I added 2 new features to make
create_arrayusable for all codec configurations I could think of:shardskwarg increate_arrayto specify the index location, e.g.shards={"shape": (32, 32), "index_location": "start"}array_bytes_codeckwarg tocreate_arrayto override the array-to-bytes codec for v3 arrays, e.g. to specify the endianness.