-
Notifications
You must be signed in to change notification settings - Fork 28
Added get_config method and repr support to all filters; Add parameter properties to some filters
#373
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
base: main
Are you sure you want to change the base?
Conversation
158889b to
b08012d
Compare
b08012d to
2bd766c
Compare
get_config method and repr support to all filters; Add parameter properties to some filters
2bd766c to
d63b597
Compare
src/hdf5plugin/_filters.py
Outdated
| filter_id: int | ||
| filter_name: str | ||
|
|
||
| def _init( |
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.
Do not use __init__ to avoid providing a typed signature for __init__ since all inherited classes have a different __init__..
Not so nice but working
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.
Fine having _init else to not have it typed you can have something very generic like __init__(self, *args, **kwargs) maybe ?
| @property | ||
| def nelems(self) -> int: | ||
| """Number of elements per block""" | ||
| return self.filter_options[0] | ||
|
|
||
| @property | ||
| def cname(self) -> Bitshuffle._CNameType: | ||
| """Compressor name""" | ||
| return _cname_from_id(self.filter_options[1], self.__COMPRESSIONS) | ||
|
|
||
| @property | ||
| def clevel(self) -> int | None: | ||
| """Compression level, only for `zstd` compressor, None for others""" | ||
| return self.filter_options[2] if self.cname == "zstd" else None |
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.
Adding properties for the "simple" filter_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.
if compressor is zstd are we sure filter_options with have at least 3 elements ?
| with self.subTest(filter=filter_class.filter_name): | ||
| filter_instance = filter_class() | ||
| repr_string = repr(filter_instance) | ||
| repr_instance = eval( |
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.
Keeping this test here so it is not installed since it uses eval.
| @property | ||
| def nelems(self) -> int: | ||
| """Number of elements per block""" | ||
| return self.filter_options[0] |
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.
filter_options can be empty I think (default value is an empty tuple).
src/hdf5plugin/_filters.py
Outdated
| filter_id: int | ||
| filter_name: str | ||
|
|
||
| def _init( |
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.
Fine having _init else to not have it typed you can have something very generic like __init__(self, *args, **kwargs) maybe ?
| @property | ||
| def nelems(self) -> int: | ||
| """Number of elements per block""" | ||
| return self.filter_options[0] | ||
|
|
||
| @property | ||
| def cname(self) -> Bitshuffle._CNameType: | ||
| """Compressor name""" | ||
| return _cname_from_id(self.filter_options[1], self.__COMPRESSIONS) | ||
|
|
||
| @property | ||
| def clevel(self) -> int | None: | ||
| """Compression level, only for `zstd` compressor, None for others""" | ||
| return self.filter_options[2] if self.cname == "zstd" else None |
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.
if compressor is zstd are we sure filter_options with have at least 3 elements ?
This MR adds API to retrieve the configuration of the filter in a more explicit way than through the
filter_optionstuple of ints.get_config() -> dictmethods (to do the same asnumcodecs) to all filters which returns the kwargs to pass to the given filter to instantiate one with the same configuration.@propertyto give access to those parameters to some filters. This is not done for Zfp, Sperr, SZ and SZ3 where the parameter schema is a bit more complex.reprcloses #371
closes #370