Update stubtest for async and dunder pos only checking #7333
Conversation
This comment has been minimized.
This comment has been minimized.
| @@ -236,15 +236,15 @@ class AsyncGeneratorType(AsyncGenerator[_T_co, _T_contra]): | |||
| __name__: str | |||
| __qualname__: str | |||
| def __aiter__(self) -> AsyncGeneratorType[_T_co, _T_contra]: ... | |||
| async def __anext__(self) -> _T_co: ... | |||
| async def asend(self, __val: _T_contra) -> _T_co: ... | |||
| def __anext__(self) -> Coroutine[Any, Any, _T_co]: ... | |||
There was a problem hiding this comment.
Out of curiosity, what is the reason for these changes?
There was a problem hiding this comment.
Well,
>>> inspect.iscoroutinefunction(types.AsyncGeneratorType.aclose)
False
I'm not sure if this matters at all, cc @AlexWaygood who added this to stubtest.
There was a problem hiding this comment.
cc. @AlexWaygood who made them async only a few weeks ago as well...
There was a problem hiding this comment.
"Classes" in the types module are weird, I'm happy to go with whatever stubtest thinks best personally.
There was a problem hiding this comment.
(the other big change we've made recently is switching a lot of (...) -> Awaitable into async (...) -> which actually does make a difference to type checking)
There was a problem hiding this comment.
Interestingly, it looks like there are already at least 2 open BPO issues about the fact that inspect.iscoroutinefunction returns False for types.AsyncGeneratorType.__anext__, .asend and .aclose. https://bugs.python.org/issue42760.
I'm not really sure what we should make of that, but it's interesting!
|
There are also a ton of new stubtest failures on 3.6 and 3.7, probably because they were using |
The 3.7 ones look like true positives to me, and it doesn't look like there's too many. But yeah, it would maybe be good to have a command-line option to switch the |
This comment has been minimized.
This comment has been minimized.
tests/stubtest_allowlists/py37.txt
Outdated
| asyncio.AbstractEventLoop.run_in_executor | ||
| asyncio.events.AbstractEventLoop.run_in_executor |
There was a problem hiding this comment.
This is a true positive -- it's async in the 3.7 branch of CPython; it was changed to be a synchronous method in 3.8. I think we should fix the stub rather than adding it to the allowlist. https://github.com/python/cpython/blob/811f65ba263140b6ba28151246b52efe149a6382/Lib/asyncio/events.py#L289
There was a problem hiding this comment.
I think we should still allowlist it. If you've implemented the AbstractEventLoop interface, you probably don't want some type checker yelling at you for returning a Future (if you check with 3.7).
There was a problem hiding this comment.
Maybe we could add a comment to that effect?
This comment has been minimized.
This comment has been minimized.
3 similar comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
|
Hopefully just in time for the daily test to tell us if this causes issues with third party stubs :-) |
|
@hauntsaninja, would you be up for trying this again now that #7352 is merged? The |
No description provided.