Skip to content

Conversation

brianschubert
Copy link
Member

@brianschubert brianschubert commented Nov 26, 2024

Fixes #18009, fixes #19282, fixes #17181

Modelling the runtime behavior of isinstance (which erases generic type arguments) isn't applicable to TypeIs. This PR adds a flag so that we can skip that logic deep inside conditional_types_with_intersection.

It's a little awkward having to pass a flag down through so many call levels, but I can't think of a better way.

This comment has been minimized.

@brianschubert brianschubert marked this pull request as draft November 26, 2024 21:20

This comment has been minimized.

This comment has been minimized.

@brianschubert brianschubert marked this pull request as ready for review June 12, 2025 13:50
@sterliakov
Copy link
Collaborator

More issues might be closed by this PR: sterliakov/mypy-issues#70 I didn't look deeper, but all four look related enough

This comment has been minimized.

Copy link
Collaborator

@hauntsaninja hauntsaninja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! I made some small additions. We have a new hit in starlette, but I think this is correct?

Edit: see also #19320

Copy link
Contributor

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

pytest (https://github.com/pytest-dev/pytest)
+ testing/test_monkeypatch.py:420: error: Unused "type: ignore" comment  [unused-ignore]

starlette (https://github.com/encode/starlette)
+ starlette/middleware/errors.py:178: error: Argument 1 to "run_in_threadpool" has incompatible type "Callable[[Request, Exception], Response | Awaitable[Response]] | Callable[[WebSocket, Exception], Awaitable[None]]"; expected "Callable[[Request, Exception], Response]"  [arg-type]
+ starlette/_exception_handler.py:61: error: Argument 1 to "run_in_threadpool" has incompatible type "Callable[[Request, Exception], Response | Awaitable[Response]] | Callable[[WebSocket, Exception], Awaitable[None]]"; expected "Callable[[Request | WebSocket, Exception], Any]"  [arg-type]
+ starlette/routing.py:68: error: Incompatible types in assignment (expression has type "Callable[..., Awaitable[Any]] | partial[Coroutine[Any, Any, Awaitable[Response] | Response]]", variable has type "Callable[[Request], Awaitable[Response]]")  [assignment]
+ starlette/routing.py:68: error: Too few arguments for "run_in_threadpool"  [call-arg]

@hauntsaninja hauntsaninja merged commit 6886b7a into python:master Jun 21, 2025
19 checks passed
@brianschubert
Copy link
Member Author

Just confirming (a little late), the starlette hit looks correct to me too.

I think it boils down to something like this:

from collections.abc import Callable
from typing import Any
from typing_extensions import TypeIs

class Foo[T]: pass

def is_foo_factory(x: object) -> TypeIs[Callable[..., Foo[Any]]]: ...

# ===== Before  =====

def f(x: Callable[..., int | Foo[int]] | Callable[..., Foo[str]]) -> None:
    if is_foo_factory(x):
        reveal_type(x)  # N: Revealed type is "def (*Any, **Any) -> SCRATCH.Foo[Any]"
    else:
        reveal_type(x)  # E: Statement is unreachable 


# ===== After  =====

def f(x: Callable[..., int | Foo[int]] | Callable[..., Foo[str]]) -> None:
    if is_foo_factory(x):
        reveal_type(x)  # N: Revealed type is "def (*Any, **Any) -> SCRATCH.Foo[Any]"
    else:
        reveal_type(x)  # N: Revealed type is "def (*Any, **Any) -> builtins.int | SCRATCH.Foo[builtins.int]"

which all looks right to me. From what I can tell, the logic in starlette is expecting the else branch to also narrow Callable[..., int | Foo[int]] to Callable[..., int], which wouldn't be right.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants