-
Notifications
You must be signed in to change notification settings - Fork 96
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
(chore) Type hints for abc codec codec_id
attribute
#702
base: main
Are you sure you want to change the base?
Conversation
codec_id
attribute
i'm just annotating the type of the |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #702 +/- ##
=======================================
Coverage 99.96% 99.96%
=======================================
Files 63 63
Lines 2754 2754
=======================================
Hits 2753 2753
Misses 1 1
|
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.
👍 , just one suggestion, feel free to take or leave and then merge
@@ -34,8 +34,8 @@ def test_all_classes_registered(): | |||
if ( | |||
inspect.isclass(obj) | |||
and issubclass(obj, numcodecs.abc.Codec) | |||
and hasattr(obj, 'codec_id') |
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.
and hasattr(obj, 'codec_id') | |
and obj is not numcodecs.abc.Codec |
I think is a bit clearer in intent (we don't want or need the base codec to be registered)?
I'm not sure how to type the buffer parameters. As long as we have a
pickle
codec, I don't think it's possible to restrict the return type ofdecode
to anything narrower thanobject
.