Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 1.9k
Change create_task type#6779
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
Change create_task type #6779
Uh oh!
There was an error while loading. Please reload this page.
Conversation
| defcreate_task(coro: Generator[_TaskYieldType, None, _T] |Awaitable[_T], *, name: str|None= ...) ->Task[_T]: ... | ||
| defcreate_task(coro: Generator[Any, None, _T] |Coroutine[Any, Any, _T], *, name: str|None= ...) ->Task[_T]: ... | ||
| else: | ||
| defcreate_task(coro: Generator[_TaskYieldType, None, _T] |Awaitable[_T]) ->Task[_T]: ... |
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.
Notice that _TaskYieldType was not used.
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.
This doesn't seem right. Event loops typically use yields to communicate between tasks and the loop, so the generator must yield objects of a specific 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.
Some experiments:
importasynciodefgen(): i=yieldprint(1, i) # prints: `1, None`return'a'asyncdefmain(): print('res', awaitasyncio.create_task(gen())) # prints: `res, a`asyncio.run(main())And:
importasynciodefgen(): i=yieldprint(1, i) # prints: `1, None`yield'a'asyncdefmain(): print('res', awaitasyncio.create_task(gen())) asyncio.run(main())1 None Traceback (most recent call last): File "/Users/sobolev/Desktop/cpython/build/ex.py", line 10, in <module> asyncio.run(main()) File "/Users/sobolev/.pyenv/versions/3.9.9/lib/python3.9/asyncio/runners.py", line 44, in run return loop.run_until_complete(main) File "/Users/sobolev/.pyenv/versions/3.9.9/lib/python3.9/asyncio/base_events.py", line 642, in run_until_complete return future.result() File "/Users/sobolev/Desktop/cpython/build/ex.py", line 8, in main print('res', await asyncio.create_task(gen())) # prints: `res, a` File "/Users/sobolev/Desktop/cpython/build/ex.py", line 5, in gen yield 'a' RuntimeError: Task got bad yield: 'a' This comment has been minimized.
This comment has been minimized.
sobolevn commented Jan 2, 2022
Ok, we have some problems: self._navigationPromise=self._loop.create_task(asyncio.wait([ self._lifecycleCompletePromise, self._createTimeoutPromise(), ], return_when=concurrent.futures.FIRST_COMPLETED))Here |
Akuli commented Jan 2, 2022
Doesn't look like a |
sobolevn commented Jan 2, 2022 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
@Akuli sorry, I had to clarify that I'm talking about typeshed's definition: typeshed/stdlib/asyncio/tasks.pyi Lines 235 to 239 in 505ea72
|
Akuli commented Jan 2, 2022
A few observations:
|
sobolevn commented Jan 2, 2022 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Ok, I have no idea what is going on:
|
sobolevn commented Jan 2, 2022
Ok, my plan is to sync all |
hauntsaninja commented Jan 2, 2022
Probably shouldn't merge this as is, given the false positives. The one in my code could be potentially worked around by changing Re stubtest: |
sobolevn commented Jan 2, 2022
Totally do not merge this as-is, I will spend several days fixing all the |
srittau commented Jan 2, 2022
I've converted this to a draft for now. Please mark as "ready for review" when it is. :) |
This comment has been minimized.
This comment has been minimized.
sobolevn commented Jan 25, 2022 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
I've changed all things I could find to be typeshed/stdlib/asyncio/tasks.pyi Lines 235 to 239 in 7bc9c16
But, mypy-primer found some new ones 🤔 |
sobolevn commented Jan 25, 2022 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Analysis:
This error does not look good to me: https://github.com/Yelp/paasta/blob/5e504597ea5a2af952a12785808e523b53b7c01d/paasta_tools/instance/kubernetes.py#L545 🤔 Aha! Found the reason: the decorator annotation is not quite correct: https://github.com/Yelp/paasta/blob/5e504597ea5a2af952a12785808e523b53b7c01d/paasta_tools/async_utils.py#L93-L97 |
srittau commented Feb 2, 2022
@sobolevn If I understand correctly, in your opinion the primer diffs all show genuine problems? |
sobolevn commented Feb 2, 2022
srittau commented Feb 2, 2022
Ok, I'll have a look once the merge conflicts are resolved. |
Diff from mypy_primer, showing the effect of this PR on open source code: core (https://github.com/home-assistant/core) + homeassistant/core.py:439: error: Argument 1 to "create_task" of "AbstractEventLoop" has incompatible type "Awaitable[_R]"; expected "Union[Coroutine[Any, Any, _R], Generator[Any, None, _R]]" [arg-type]+ homeassistant/core.py:470: error: Need type annotation for "task" [var-annotated]+ homeassistant/core.py:470: error: Argument 1 to "create_task" of "AbstractEventLoop" has incompatible type "Awaitable[_R]"; expected "Union[Coroutine[Any, Any, <nothing>], Generator[Any, None, <nothing>]]" [arg-type] paasta (https://github.com/yelp/paasta) - paasta_tools/instance/kubernetes.py:553: error: Argument 1 to "append" of "list" has incompatible type "Task[Sequence[Any]]"; expected "Future[Dict[str, Any]]"+ paasta_tools/instance/kubernetes.py:545: error: Need type annotation for "pods_task"+ paasta_tools/instance/kubernetes.py:546: error: Argument 1 to "create_task" has incompatible type "Awaitable[Sequence[Any]]"; expected "Union[Generator[Any, None, <nothing>], Coroutine[Any, Any, <nothing>]]"+ paasta_tools/instance/kubernetes.py:1017: error: Need type annotation for "pods_task"+ paasta_tools/instance/kubernetes.py:1018: error: Argument 1 to "create_task" has incompatible type "Awaitable[Sequence[Any]]"; expected "Union[Generator[Any, None, <nothing>], Coroutine[Any, Any, <nothing>]]"+ paasta_tools/instance/kubernetes.py:1202: error: Need type annotation for "pods_task"+ paasta_tools/instance/kubernetes.py:1203: error: Argument 1 to "create_task" has incompatible type "Awaitable[Sequence[Any]]"; expected "Union[Generator[Any, None, <nothing>], Coroutine[Any, Any, <nothing>]]" |
| fromcollections.abcimportIterable | ||
| fromsocketimportAddressFamily, SocketKind, _Address, _RetAddress, socket | ||
| fromtypingimportIO, Any, Awaitable, Callable, Generator, Sequence, TypeVar, Union, overload | ||
| fromtypingimportIO, Any, Awaitable, Callable, Coroutine, Generator, Sequence, TypeVar, Union, overload |
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.
Nit for future reference: Coroutine should now be imported from collections.abc.
sobolevn commented Feb 2, 2022
Thanks everyone 🎉 |
I've manually tested that
Awaitables are not supported:Output:
But,
Generatorobjects are also supported:Closes#6770