-
Notifications
You must be signed in to change notification settings - Fork 0
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
Fix type errors caught by pyright but not mypy #156
Conversation
Codecov Report
@@ Coverage Diff @@
## master #156 +/- ##
==========================================
- Coverage 94.50% 94.42% -0.08%
==========================================
Files 44 44
Lines 1292 1310 +18
==========================================
+ Hits 1221 1237 +16
- Misses 71 73 +2
|
1b3ff04
to
5271a2b
Compare
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.
Looks good, only a few minor comments.
@@ -21,7 +21,7 @@ def __init__( | |||
self, | |||
host: str = "127.0.0.1", | |||
port: int = 5555, | |||
socket_factory: Optional[SocketFactory] = create_zmq_push_socket, | |||
socket_factory: SocketFactory = create_zmq_push_socket, |
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.
Good spot, should probably also do this in the base class
There are still a few (6) errors remaining but mostly around the interpreters which I'd like to overhaul slightly anyway |
The class variables are types rather than instances in their own right so should be typed as such. This allows type checkers to correctly check key lookups.
Iterators are iterable but the more specific type allows callers to use additional methods.
Methods are generated dynamically so static type checking can't work.
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.
Looks good, thank you for that. Once the zeromq_push_device
comment has been fixed I think you're good to go. Do you want me to change the mypy to pyright in the checks in here, or should we do a different pr for it?
Up to you but I think I'd go for a separate PR so it can be rolled back more easily if there's a pyright problem we can't work around. Everything in this PR should still be valid if we stick with mypy. |
Agree. In which case either resolve the issue in the push device or make a new issue for it, and merge this :) |
|
type
as type of Inputs/Outputs fieldsWhere type checking has been ignored, comments have been added to explain
why the code is still valid.