Add typing annotations for can.notifier #679
Conversation
This adds typing annotations for functions in can.notifier. In addition, this remove the redundant typing information that was previously in the docstring, since we now have sphinx-autodoc-typehints to generate the types for the docs from the annotations in the function signature. This works towards PEP 561 compatibility.
| if ( | ||
| self._loop is not None | ||
| and hasattr(bus, "fileno") | ||
| and bus.fileno() >= 0 # type: ignore |
There was a problem hiding this comment.
This is disabled here because mypy doesn't really deal well with hasattr checks. See the discussion here. We could either:
- Disable the type checker (like is done here)
- Perform the cast to
Any - Refactor the code to have
filenoon the base class returnNoneby default (which we can check for), and then subclasses provide their own implementation as needed
I'm personally inclined to 3, but that's a bit more involved than the other 2 (and perhaps should be kept out of this change). Thoughts?
There was a problem hiding this comment.
I vote for 3, but it can return -1 instead of None.
There was a problem hiding this comment.
I'd vote for 1. It's just a shortcoming of the type system, and no problem with our API.
There was a problem hiding this comment.
I agree that one makes sense for this PR, but 3 would be the best long term solution.
Codecov Report
@@ Coverage Diff @@
## develop #679 +/- ##
===========================================
+ Coverage 68.74% 68.75% +<.01%
===========================================
Files 69 69
Lines 6235 6240 +5
===========================================
+ Hits 4286 4290 +4
- Misses 1949 1950 +1 |
Codecov Report
@@ Coverage Diff @@
## develop #679 +/- ##
===========================================
+ Coverage 68.74% 68.75% +<.01%
===========================================
Files 69 69
Lines 6235 6240 +5
===========================================
+ Hits 4286 4290 +4
- Misses 1949 1950 +1 |
This adds typing annotations for functions in can.notifier.
In addition, this remove the redundant typing information that was
previously in the docstring, since we now have sphinx-autodoc-typehints
to generate the types for the docs from the annotations in the function
signature.
This works towards PEP 561 compatibility.