-
-
Notifications
You must be signed in to change notification settings - Fork 324
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
Array creation with strings for filters, serializer, compressors #2839
base: main
Are you sure you want to change the base?
Array creation with strings for filters, serializer, compressors #2839
Conversation
_compressor = parse_compressor(compressor) | ||
|
||
if filters is None: | ||
_filters = None | ||
elif filters == "auto": | ||
_filters = default_filters | ||
else: | ||
if isinstance(filters, Iterable): |
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 is also checked in parse_filters
, so I removed it
@@ -4264,24 +4264,13 @@ def _parse_chunk_encoding_v2( | |||
elif isinstance(compressor, tuple | list) and len(compressor) == 1: | |||
_compressor = parse_compressor(compressor[0]) | |||
else: | |||
if isinstance(compressor, Iterable) and not isinstance(compressor, dict): |
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.
same
@@ -1124,6 +1149,30 @@ async def test_v2_chunk_encoding( | |||
assert arr.compressors == compressor_expected | |||
assert arr.filters == filters_expected | |||
|
|||
@staticmethod |
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.
Can you please add another test, where a codec would need mandatory configuration params to fail with just a string?
TODO:
docs/user-guide/*.rst
changes/