asyncio, inspect: use static types in TypeIs#15931
Conversation
The set-theoretically correct way to write these TypeIs functions is with a type that includes all instances of the type they are narrowing to; for example "Awaitable[Any]" means "some unknown set of awaitables"; "Awaitable[object]" means "all awaitables, regardless of what they await to". We can't spell this type for cases involving invariant type parameters or callable parameter sets, so leave those alone for now. This may cause new type checker diagnostics because it will lead type checkers to often infer more precise types. Those errors are at least in theory correct, but let's see how big the fallout is.
# Conflicts: # stdlib/inspect.pyi
This comment has been minimized.
This comment has been minimized.
|
(Will post back with results from going over mypy-primer.) |
|
I previously tried a variant of this in #14784 and posted some primer analysis in that thread, if that helps |
|
Conclusions:
Below is Codex's summary of the changes, with links to the relevant code: DetailsAnalysis of mypy-primer fallout from #15931This is an analysis of the mypy-primer output in The PR changes several High-level summaryMost of the fallout is from code using the common helper pattern: T | Awaitable[T]and then narrowing with However, several primer results appear to be mypy limitations rather than real
Alex's earlier related analysis on #14784 is relevant:
Classification by projectpsycopgProject: https://github.com/psycopg/psycopg Affected code: Likely classification: mostly true positive / exposed unsoundness. The code has a helper that accepts a callable returning Typical workaround: from collections.abc import Awaitable
from inspect import isawaitable
from typing import TypeVar, cast
T = TypeVar("T")
async def await_maybe(value: T | Awaitable[T]) -> T:
if isawaitable(value):
return await cast(Awaitable[T], value)
return cast(T, value)For a public API, overloads give better call-site inference. aiohttp-devtoolsProject: https://github.com/aio-libs/aiohttp-devtools Affected code: Likely classification: true positive or annotation too broad. The annotation permits aiohttpProject: https://github.com/aio-libs/aiohttp Affected code: Likely classification: mostly harmless locally, but still points at a real The code checks PrefectProject: https://github.com/PrefectHQ/prefect Affected code:
Likely classification: mixed. The The result = acquire_concurrency_slots(...)
assert not asyncio.iscoroutine(result)
limits = resultAfter the assertion, mypy should narrow away the coroutine arm. With the PR Minimal repro: from collections.abc import Coroutine
from typing import Any
from typing_extensions import Never, TypeIs
def is_coro(x: object) -> TypeIs[Coroutine[object, Never, object]]:
return False
def needs_list(x: list[int]) -> None: ...
def f(x: list[int] | Coroutine[Any, Any, list[int]]) -> None:
assert not is_coro(x)
reveal_type(x) # list[int] | Coroutine[Any, Any, list[int]]
needs_list(x) # should be OK, but errorsWith the old gradual predicate: def is_coro(x: object) -> TypeIs[Coroutine[Any, Any, Any]]:
return Falsemypy narrows correctly to This appears not to be primarily about the contravariant type parameter. A from typing import Any, Generic, TypeVar
from typing_extensions import Never, TypeIs
T_contra = TypeVar("T_contra", contravariant=True)
class Sink(Generic[T_contra]): ...
def is_sink(x: object) -> TypeIs[Sink[Never]]:
return False
def f(x: int | Sink[Any]) -> None:
assert not is_sink(x)
reveal_type(x) # intBut a covariant top-materialization case fails: from typing import Any, Generic, TypeVar
from typing_extensions import TypeIs
T_co = TypeVar("T_co", covariant=True)
class Source(Generic[T_co]): ...
def is_source(x: object) -> TypeIs[Source[object]]:
return False
def f(x: int | Source[Any]) -> None:
assert not is_source(x)
reveal_type(x) # int | Source[Any]So the relevant mypy bug seems to be negative For if TYPE_CHECKING:
assert iscoroutine(coro)
infrastructure = await coroThis is a likely false positive / type-checker workaround becoming stale. The steam.pyProject: https://github.com/Gobot1234/steam.py Affected code: Likely classification: true positive / exposed unsoundness. This is another strawberryProject: https://github.com/strawberry-graphql/strawberry Affected code:
Likely classification: mixed.
hydra-zenProject: https://github.com/mit-ll-responsible-ai/hydra-zen Affected code: Likely classification: true positive, according to Alex's previous analysis.
scrapyProject: https://github.com/scrapy/scrapy Affected code:
Likely classification: mixed.
discord.pyProject: https://github.com/Rapptz/discord.py Affected code: Likely classification: true positive / exposed unsoundness. This is another helper returning panderaProject: https://github.com/pandera-dev/pandera Affected code:
Likely classification: mixed. The unused ignore is a true positive / cleanup. The So that part is a false positive from mypy's perspective, or at least not a pytestProject: https://github.com/pytest-dev/pytest Affected code: Likely classification: false positive. The test intentionally monkeypatches trioProject: https://github.com/python-trio/trio Affected code: Likely classification: true positive in the type-theoretic sense, per Alex's
Similarly, after Alex's previous analysis: anyioProject: https://github.com/agronholm/anyio Affected code: Likely classification: true positive / exposed unsoundness. The callable can return kopfProject: https://github.com/nolar/kopf Affected code: Likely classification: true positive / exposed missing annotation information. The code calls a method whose result is graphql-coreProject: https://github.com/graphql-python/graphql-core Affected code: Likely classification: mypy limitation / false positive. Alex's previous analysis applies here. The result of Previous analysis: paastaProject: https://github.com/yelp/paasta Affected code: Likely classification: mixed / ergonomics issue. The code closes the input only when it is a coroutine: if inspect.iscoroutine(async_fn_or_awaitable):
async_fn_or_awaitable.close()At runtime this is legitimate: coroutine objects have |
|
Diff from mypy_primer, showing the effect of this PR on open source code: strawberry (https://github.com/strawberry-graphql/strawberry)
+ strawberry/types/object_type.py:295: error: Value of type variable "T" of "_inject_default_for_maybe_annotations" cannot be "object" [type-var]
+ strawberry/types/object_type.py:296: error: Value of type variable "T" of "_wrap_dataclass" cannot be "object" [type-var]
hydra-zen (https://github.com/mit-ll-responsible-ai/hydra-zen)
+ src/hydra_zen/third_party/beartype.py:125: error: Cannot assign to a method [method-assign]
scrapy (https://github.com/scrapy/scrapy)
+ scrapy/cmdline.py:70: error: Incompatible types in assignment (expression has type "object", target has type "ScrapyCommand") [assignment]
pandera (https://github.com/pandera-dev/pandera)
+ pandera/dtypes.py:568: error: Unused "type: ignore" comment [unused-ignore]
+ tests/pandas/test_dtypes.py:306: error: Item "object" of "Any | object" has no attribute "foo" [union-attr]
pytest (https://github.com/pytest-dev/pytest)
+ testing/test_monkeypatch.py:439: error: Statement is unreachable [unreachable]
trio (https://github.com/python-trio/trio)
+ src/trio/_core/_tests/test_ki.py:682: error: Argument 1 to "_consume_async_generator" has incompatible type "AsyncGeneratorType[object, Never]"; expected "AsyncGenerator[None, None]" [arg-type]
+ src/trio/_core/_tests/test_ki.py:687: error: Argument 1 to "send" of "GeneratorType" has incompatible type "None"; expected "Never" [arg-type]
graphql-core (https://github.com/graphql-python/graphql-core)
+ tests/execution/test_middleware.py:265: error: "object" has no attribute "data" [attr-defined]
+ tests/execution/test_middleware.py:267: error: "object" has no attribute "data" [attr-defined]
|
|
Primer analysis:
|
Didn't look in detail at the rest, but LGTM. I wouldn't mind to see the async changes in a separate PR, so we could have another look at the primer failures in isolation. On a first glance they looked acceptable to me. |
3 participants
The set-theoretically correct way to write these TypeIs functions is
with a type that includes all instances of the type they are narrowing to;
for example "Awaitable[Any]" means "some unknown set of awaitables";
"Awaitable[object]" means "all awaitables, regardless of what they await to".
We can't spell this type for cases involving invariant type parameters or
callable parameter sets, so leave those alone for now.
This may cause new type checker diagnostics because it will lead type
checkers to often infer more precise types. Those errors are at least
in theory correct, but let's see how big the fallout is.