Skip to content
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

type inference is fundementally broken when dealing with class hierarchies #18737

Open
justinvanwinkle opened this issue Feb 25, 2025 · 11 comments
Labels
bug mypy got something wrong

Comments

@justinvanwinkle
Copy link

Bug Report

mypy gives an incorrect list of possible types. Although B is a subclass of A, B is not A.

To Reproduce

from typing import reveal_type
import random


class A:
    pass


class B(A):
    pass


class C(B):
    pass


def foo() -> A | B | C:
    return random.choice([A, B, C])()


def main():
    x: A | B | C = foo()

    reveal_type(x)  # mypy Revealed type is "Union[mypy_ex.A, mypy_ex.B, mypy_ex.C]"  (pyright says A | B | C)
    if isinstance(x, C):
        print("aborting")
        return
    reveal_type(x)  # mypy Revealed type is "mypy_ex.A" (pyright correctly says A | B)


if __name__ == "__main__":
    for _ in range(10):
        main()


"""
output from running:
Runtime type is 'B'
Runtime type is 'B'
Runtime type is 'C'
aborting
Runtime type is 'B'
Runtime type is 'B'
Runtime type is 'C'
aborting
Runtime type is 'A'
Runtime type is 'A'
Runtime type is 'A'
Runtime type is 'A'
Runtime type is 'A'
Runtime type is 'A'
Runtime type is 'B'
Runtime type is 'B'
Runtime type is 'B'
Runtime type is 'B'
Runtime type is 'B'
Runtime type is 'B'
"""

Expected Behavior

Actual Behavior

Your Environment

  • Mypy version used: 1.15
  • Mypy command-line flags: mypy --check-untyped-defs --config-file=/dev/null mypy_ex.py
  • Mypy configuration options from mypy.ini (and other config files):
  • Python version used:
@justinvanwinkle justinvanwinkle added the bug mypy got something wrong label Feb 25, 2025
@justinvanwinkle
Copy link
Author

ok if that wasn't obviously a bug enough (because technically a B is an A:

class A:
    def a_say(self):
        print('a')


class B(A):
    def b_say(self):
        print('b')


class C(B):
    def c_say(self):
        print('c')


def foo() -> A | B | C:
    return random.choice([A, B, C])()


def main():
    x: A | B | C = foo()

    reveal_type(x)  # mypy Revealed type is "Union[mypy_ex.A, mypy_ex.B, mypy_ex.C]"

    if hasattr(x, "b_say"):
        x.b_say()
        reveal_type(x)  # mypy Revealed type is "mypy_ex.A"```

@justinvanwinkle
Copy link
Author

justinvanwinkle commented Feb 25, 2025

Similar 'degrade to common superclass' behavior where it directly breaks something.

class B:
    pass

class E:
    pass


def foo() -> B | E:
    return random.choice([B, E])()  # error: Incompatible return value type (got "object", expected "B | E")```

@JelleZijlstra
Copy link
Member

You are reporting a number of different samples and I'm not sure there's anything actionable here.

Your first post says Revealed type is "mypy_ex.A" (pyright correctly says A | B). But since B is a subclass of A and Python type annotations include subclasses, A | B and A are equivalent types; they include the same values. Neither type checker is wrong.

The second post shows the most confusing behavior and perhaps there's something we can change there. Here's a mypy playground with a few more reveal_types: https://mypy-play.net/?mypy=latest&python=3.12&gist=e62cbd2edd0a255ffe19604e5ff4f79d. I think the hasattr check makes mypy do two things: it simplifies the union (A | B | C to the equivalent type A) and it marks the object as having an attribute b_say of unknown type. Should it narrow to B instead? No, because there may be other subclasses of A that also have an attribute b_say with another type. However, maybe it should narrow it to something like Callable[[], None] | Any.

The third post is an example of #12056.

@justinvanwinkle
Copy link
Author

justinvanwinkle commented Feb 25, 2025

Hi JelleZilstra, thanks for looking at this.

I understand that B is a subclass of A, so a B 'is' an A. However under that logic just always infer the type object and you can technically close all bugs going forward as it is always 'correct' with that definition of correctness (I'm being glib but that is the same argument and in fact what it does do in the 3rd example).

I would propose that any time a static analyzer could logically infer a type, but fails to do so, that is a defect. The goal has to be to get as close to the type revealed at runtime as possible. In all my examples a it is relatively trivial to determine the correct type. All Mypy has to do is not unify to the superclass so aggressively, and in all these cases there's no logical reason for it to unify at all.

@justinvanwinkle
Copy link
Author

justinvanwinkle commented Feb 25, 2025

I think the hasattr check makes mypy do two things: it simplifies the union (A | B | C to the equivalent type A) and it marks the object as having an attribute b_say of unknown type. Should it narrow to B instead? No, because there may be other subclasses of A that also have an attribute b_say with another type. However, maybe it should narrow it to something like Callable[[], None] | Any.

In that case here is another bug. As you said, you can't narrow to B because there may be an unknown type somewhere that also matches. Here B has the Protocol BSay, but there could be any number of other subclasses of A that also support BSay so narrowing is incorrect. I think you have to pick one or the other of these as wrong because the logic is identical.

@JelleZijlstra

@runtime_checkable
class BSay(Protocol):
    def b_say(self): ...


class A:
    def a_say(self):
        print("a")


class B(A):
    def b_say(self):
        print("b")


class C(B):
    def c_say(self):
        print("c")


def foo() -> A | B | C:
    return random.choice([A, B, C])()


def main():
    x: A | B | C = foo()

    reveal_type(x)  # mypy Revealed type is "Union[mypy_ex.A, mypy_ex.B, mypy_ex.C]"

    if isinstance(x, BSay):
        x.b_say()
        reveal_type(x)  # Revealed type is "mypy_ex.B"

@justinvanwinkle
Copy link
Author

justinvanwinkle commented Feb 25, 2025

The more I play with this, the more it's obvious that this is not a philosophical decision. Mypy seems to 'panic' in lots of situations and lose track of the Union.

There's no theory of types that makes the following make sense:

def main():
    x: A | B | C = foo()

    reveal_type(x)  # mypy Revealed type is "Union[mypy_ex.A, mypy_ex.B, mypy_ex.C]"

    if hasattr(x, "a_say"):
        reveal_type(x)  # Revealed type is "Union[mypy_ex.A, mypy_ex.B, mypy_ex.C]"

    if hasattr(x, "a_say") or hasattr(x, "__dict__"):
        reveal_type(x)  # Revealed type is "mypy_ex.A"

    if hasattr(x, "a_say"):
        reveal_type(x)  # Revealed type is "Union[mypy_ex.A, mypy_ex.B, mypy_ex.C]"

@justinvanwinkle
Copy link
Author

Here's another one. Why does having ASay let you be A | B | C, but BSay is not B | C? This is silly and obviously inconsistent and a bug.

@runtime_checkable
class ASay(Protocol):
    def a_say(self): ...


@runtime_checkable
class BSay(Protocol):
    def b_say(self): ...


def main():
    x: A | B | C = foo()

    reveal_type(x)  # Revealed type is "Union[mypy_ex.A, mypy_ex.B, mypy_ex.C]"

    if isinstance(x, ASay):
        reveal_type(x)  # Revealed type is "Union[mypy_ex.A, mypy_ex.B, mypy_ex.C]"

    if isinstance(x, BSay):
        reveal_type(x)  # Revealed type is "mypy_ex.B"

    if isinstance(x, ASay):
        reveal_type(x)  # Revealed type is "Union[mypy_ex.A, mypy_ex.B, mypy_ex.C]"

@justinvanwinkle
Copy link
Author

Removing A from contention brings C back! 😆

import random
from typing import Protocol
from typing import reveal_type
from typing import runtime_checkable


class A:
    def a_say(self):
        print("a")


class B(A):
    def b_say(self):
        print("b")


class C(B):
    def c_say(self):
        print("c")


def foo() -> B | C:
    return random.choice([B, C])()


@runtime_checkable
class ASay(Protocol):
    def a_say(self): ...


@runtime_checkable
class BSay(Protocol):
    def b_say(self): ...


def main():
    x: B | C = foo()

    reveal_type(x)  # Revealed type is "Union[mypy_ex.B, mypy_ex.C]"

    if isinstance(x, ASay):
        reveal_type(x)  # Revealed type is "Union[mypy_ex.B, mypy_ex.C]"

    if isinstance(x, BSay):
        reveal_type(x)  # Revealed type is "Union[mypy_ex.B, mypy_ex.C]"

    if isinstance(x, ASay):
        reveal_type(x)  # Revealed type is "Union[mypy_ex.B, mypy_ex.C]"


if __name__ == "__main__":
    for _ in range(10):
        main()```

@justinvanwinkle
Copy link
Author

justinvanwinkle commented Feb 25, 2025

Ok not to just nuke this from orbit, but:

def main():
    x: B | C = foo()

    reveal_type(x)  # Revealed type is "Union[mypy_ex.B, mypy_ex.C]"

    if isinstance(x, ASay):
        reveal_type(x)  # Revealed type is "Union[mypy_ex.B, mypy_ex.C]"

    if isinstance(x, BSay):
        reveal_type(x)  # Revealed type is "Union[mypy_ex.B, mypy_ex.C]"

    if hasattr(x, "a_say"):
        reveal_type(x)  # Revealed type is "Union[mypy_ex.B, mypy_ex.C]"

    if hasattr(x, "b_say"):
        reveal_type(x)  # Revealed type is "Union[mypy_ex.B, mypy_ex.C]"
def main():
    x: A | B | C = foo()

    reveal_type(x)  # Revealed type is "Union[mypy_ex.A, mypy_ex.B, mypy_ex.C]"

    if isinstance(x, ASay):
        reveal_type(x)  # Revealed type is "Union[mypy_ex.A, mypy_ex.B, mypy_ex.C]"

    if isinstance(x, BSay):
        reveal_type(x)  # Revealed type is "mypy_ex.B"

    if hasattr(x, "a_say"):
        reveal_type(x)  # Revealed type is "Union[mypy_ex.A, mypy_ex.B, mypy_ex.C]"

    if hasattr(x, "b_say"):
        reveal_type(x)  # Revealed type is "mypy_ex.A"```

@erictraut
Copy link

@justinvanwinkle, the type A | B | C and the type A are equivalent. That is, they describe exactly the same type. You can you think of types as sets. The set described by B is a pure subset of the type described by A. Likewise, the set described by C is a pure subset of the type described by B The union of A, B and C is therefore the same as A. If mypy chooses to reveal A versus A | B | C, you shouldn't think of this as "forgetting a type". It's simply reporting the equivalent type in a simpler form. You may find it useful to read this chapter of the Python typing spec. It does a good job clarifying these concepts.

There are cases where mypy uses a "join" operator rather than a "union" operator and therefore produces a type that is wider than necessary resulting in downstream false positive errors. This has been discussed in detail in other places including the summary issue that Jelle links to above. So, when you point out that mypy sometimes uses a join rather than a union, you're making a valid point, but it's one that has been discussed at length already. You're not adding anything new to the conversation. The mypy maintainers are aware of this and have been making changes over time to convert joins to unions, and they will likely continue to convert more cases over time. Each of these changes has potential backward compatibility impacts for mypy users, so they are being cautious in how they approach it.

@justinvanwinkle
Copy link
Author

justinvanwinkle commented Feb 25, 2025

In practice they aren't equivalent at all. They could be but mypy's inference treats them differently, so the right hand is disagreeing with the left hand here.

if hasattr(x, "b_say"):
        reveal_type(x)  # Revealed type is "mypy_ex.A"

This is simply incorrect. Mypy has removed the two types from the Union where it would be correct, and left the only type where it is not correct. B can be used in situations where A can be used, but A cannot always be used in situations where B can be used. I would direct you to the following article which does a good job of explaining this concept Covariance and Contravariance

If I have a function like
def uses_b(b: B): ... and I call it above, mypy will say that this is an error. If they are as equivalent as you say, it would behave the same, correct? If it determined the type to be B | C it would not be an error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mypy got something wrong
Projects
None yet
Development

No branches or pull requests

3 participants