-
Notifications
You must be signed in to change notification settings - Fork 69
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
Add PEP484 Typing + typing to setup.py #27
Conversation
cooperlees
commented
Jan 5, 2017
- Add in Py2/3 compatible PEP484 typing
- Add in typing as a install_requires in setup.py
self._channel.cancel() | ||
|
||
# TODO: Workout socket fd type |
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.
it will be an int
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.
Will change - Wasen't sure that it would always be an int
@@ -103,7 +119,9 @@ def _sock_state_cb(self, fd, readable, writable): | |||
self._timer.cancel() | |||
self._timer = None | |||
|
|||
# TODO: Workout socket fd type |
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.
int
@@ -6,7 +6,14 @@ | |||
import trollius as asyncio | |||
import pycares | |||
|
|||
from . import error | |||
from typing import ( |
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.
Sorry for my ignorance, but why are these needed if we are not really using them (just in comments) ?
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.
Those comments are evaluated by the tools performing the type checking (like mypy).
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.
) | ||
|
||
# TODO: Work out mypy no attribute error | ||
from . import error # type: ignore |
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.
Why is this import needed?
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.
Seems to be a mypy bug with this kind of import
- The import itself was already in the code
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.
Oh, right!
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.
@cooperlees What was the issue that you encountered? Seems to work fine with Python3 mypy, so I guess that this workaround should be removed.
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.
Fixes coming |
Great work, cheers! |
No worries. Thanks 👍 |