Skip to content

Conversation

@JelleZijlstra
Copy link
Member

Noticed this in mypy-primer output in #5516 on this code: https://github.com/encode/starlette/blob/master/starlette/testclient.py#L74

It calls iscoroutinefunction() on an object that may be None, which got flagged as an error but is actually fine; it just returns False.

We could also potentially use TypeGuard here, especially for iscoroutine which is just an isinstance call.

Noticed this in mypy-primer output in #5516 on this code: https://github.com/encode/starlette/blob/master/starlette/testclient.py#L74

It calls `iscoroutinefunction()` on an object that may be None, which got flagged as an error but is actually fine; it just returns False.

We could also potentially use TypeGuard here, especially for `iscoroutine` which is just an `isinstance` call.
@github-actions
Copy link
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

aiohttp (https://github.com/aio-libs/aiohttp.git)
+ aiohttp/helpers.py:117: error: Incompatible redefinition (redefinition with type "Callable[[Callable[..., Any]], bool]", original type "Callable[[object], bool]")  [misc]

@JelleZijlstra
Copy link
Member Author

That's https://github.com/aio-libs/aiohttp/blob/master/aiohttp/helpers.py#L113. Seems fine to force aiohttp to make a change here. I can send a PR to aiohttp once this PR is merged.

@srittau srittau merged commit c4e3fd9 into master May 21, 2021
@srittau srittau deleted the JelleZijlstra-patch-1 branch May 21, 2021 21:07
JelleZijlstra added a commit to JelleZijlstra/aiohttp that referenced this pull request May 21, 2021
This will be needed to make mypy pass after python/typeshed#5517 is released. The function accepts all objects, so `object` is the appropriate annotation, but `Any` is necessary for now here to make the conditional definition work with the previous type annotation.
webknjaz pushed a commit to aio-libs/aiohttp that referenced this pull request May 22, 2021
This will be needed to make mypy pass after python/typeshed#5517 is released. The function accepts all objects, so `object` is the appropriate annotation, but `Any` is necessary for now here to make the conditional definition work with the previous type annotation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants