-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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 missing asyncio 3.7 top-level functions #2320
Conversation
I don’t quite understand the error message(s)…should I protect the definition against <3.7? But you don’t test against 3.7, do you? |
Yes, you should put all new functions The Python versions that appear in Travis results are misleading; it's the Python versions we use to run the tools that perform the tests (mypy and pytype), not the Python version we check the stubs for. |
I think I got it? Sadly right after the release of mypy 0.620. :( off-topic: I wonder if there’s a way to use them and still run mypy in strict mode? |
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.
Thanks! Left a few comments.
stdlib/3/asyncio/tasks.pyi
Outdated
if sys.version_info >= (3, 7): | ||
def all_tasks(loop: Optional[AbstractEventLoop] = ...) -> Set[Task]: ... | ||
def create_task(coro: Union[Generator[Any, None, _T], Awaitable[_T]]) -> Task: ... | ||
def current_task(loop: Optional[AbstractEventLoop] = ...) -> Task: ... |
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.
Should return Optional[Task]
I think.
stdlib/3/asyncio/runners.pyi
Outdated
@@ -0,0 +1,6 @@ | |||
from typing import Awaitable, TypeVar |
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.
Should wrap the entirety of this file in sys.version_info >= (3, 7)
so that people trying to use this before 3.7 get an error.
Not sure what you are doing to get unused type-ignore warnings. You should be able to use these functions and then |
I believe I’ve addressed everything. |
(FWIW, wouldn’t it be kind of nicer to manually raise an ImportError instead of putting the whole module into a block?) |
Yes, there has been talk of adding support for that suggestion (I think on the typing issue tracker?), but currently it's not supported. Code looks good now, so I'll merge it in. Thanks for contributing! |
Here’s a first take at #2313
I couldn’t get the pytype tests running because it looks for my Python in
/opt/python/3.6/bin/
(!?) so let’s see what Travis has to say…