Skip to content

[1.16 regression] Incorrect inferred type when using a classmethod from a different class as an attribute #19146

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

Open
delfick opened this issue May 24, 2025 · 12 comments
Labels
bug mypy got something wrong

Comments

@delfick
Copy link

delfick commented May 24, 2025

Hello,

I decided to try out the new mypy 1.16 branch on the codebase I work on and I found what I think is a bug.

Let's say you have the following code (makes more sense in the concrete example it comes from)

from collections.abc import Iterable
from typing import Protocol


class GetActions(Protocol):
    def __call__(self, event_type: str) -> Iterable[str]: ...


class Parent:
    get_actions: GetActions


class ActionsGetter:
    @classmethod
    def get_actions(self, event_type: str) -> Iterable[str]:
        return ("1",)


class Child(Parent):
    get_actions = ActionsGetter.get_actions


print(Child().get_actions("one"))

This works at runtime but at static time it complains with

a.py:20: error: Incompatible types in assignment (expression has type "Callable[[], Iterable[str]]", base class "Parent" defined the type as "GetActions")  [assignment]
a.py:20: note: "GetActions.__call__" has type "Callable[[Arg(str, 'event_type')], Iterable[str]]"
Found 1 error in 1 file (checked 1 source file)

Where it believes that the type of Child.get_actions is Callable[[], Iterable[str]] instead of Callable[[Arg(str, 'event_type')], Iterable[str]]

Your Environment

  • Mypy version used: release-1.16 (revision 96525a2, built locally with mypy_mypyc-wheels for cp312-macosx_arm64)
  • Mypy command-line flags: mypy a.py
  • Mypy configuration options from mypy.ini (and other config files): no config
  • Python version used: python3.12
@delfick delfick added the bug mypy got something wrong label May 24, 2025
@A5rocks
Copy link
Collaborator

A5rocks commented May 24, 2025

Thanks, I assume we have some change that is more eagerly binding self types?

@delfick
Copy link
Author

delfick commented May 26, 2025

@A5rocks yeah, that'd be my guess too

@sterliakov
Copy link
Collaborator

I was about to blame #19025, but it isn't merged yet... Bisects to #18847.

@A5rocks
Copy link
Collaborator

A5rocks commented May 28, 2025

BTW @delfick are any of the repositories you are checking open source? It would be nice to get some more test cases for https://github.com/hauntsaninja/mypy_primer, which would let us catch these regressions earlier.

@delfick
Copy link
Author

delfick commented May 28, 2025

@A5rocks

Unfortunately not. I work on https://kraken.tech and my role specifically is around getting many millions of lines of python in our main monorepo to be strongly typed (which is made possible by mypy supporting plugins). Definitely not an open source repo!

@delfick
Copy link
Author

delfick commented May 28, 2025

@A5rocks is there a discord/slack I can join, I'm more than happy to have a closer working relationship with mypy devs!

@A5rocks
Copy link
Collaborator

A5rocks commented May 28, 2025

There's nothing user-facing, though IMO it would be a good idea to try to extend a level of support for users (and not just Dropbox!). I'm not in a position to make anything though.

Testing new releases like this is already very useful!

@delfick
Copy link
Author

delfick commented May 28, 2025

I'm not in a position to make anything though.

fair :)

Testing new releases like this is already very useful!

yeah, definitely.

It took me 1.5 years to get us onto the latest version of mypy and I'm so happy I can finally run mypy releases ahead of them being released!

I still have another 122 errors that I'm going through atm to see if I found any other regressions (this release gave me 300 new errors which is not too bad, most of them look like they're because of how mypy now understands optional Anys where it use to only think of them as Any)

@delfick
Copy link
Author

delfick commented May 28, 2025

@A5rocks after a basic run through of the remaining errors I think for this repo, it's just this issue and the one I made about the match statement, which does seem like a bad regression

@hauntsaninja hauntsaninja changed the title Incorrect inferred type when using a classmethod from a different class as an attribute in mypy 1.16 [1.16 regression] Incorrect inferred type when using a classmethod from a different class as an attribute May 28, 2025
@JukkaL
Copy link
Collaborator

JukkaL commented May 28, 2025

cc @ilevkivskyi as the author of #18847

@ilevkivskyi
Copy link
Member

OK, this one is tricky. Mypy actually never handled bound methods correctly, if you try reveal_type(ActionsGetter.get_actions), mypy will show def (str) -> Iterable[str] both before and now. And the point is for mypy this is just a regular callable, mypy has no idea it is already a bound classmethod, so Python will not bind it again when it is assigned inside class body.

To understand better, try this:

class Child:  # NOTE: base class removed
    get_actions = ActionsGetter.get_actions

reveal_type(Child().get_actions)

This will show you

error: Invalid self argument "Child" to attribute function "get_actions" with type "Callable[[str], Iterable[str]]"  [misc]
note: Revealed type is "def () -> Iterable[str]"  # NOTE: same type that appears in the error now

both in 1.15 and in 1.16. In fact, mypy follows some rules on whether a variable is a class variable or an instance variable (can't find a good docs reference on this however). And according to these rules Parent.get_actions is an instance variable, because it has an explicit non-ClassVar annotation, i.e. the type specified is already a type as it would appear on an instance. But Child.get_actions is an (implicit) class variable, so its type must be bound when accessed on an instance.

An unrelated problem is that the error message is somewhat confusing, because it says "expression has type", while in fact we are comparing "externally visible" type of attribute on subclass vs same on superclass.

So although it is technically a regression, it used to work by pure accident, and the new behavior is more consistent, i.e. uniformly broken :-). Unfortunately, a proper fix is non-trivial, it would require a new attribute on callable type, that would need to be carefully passed around everywhere (but also to be clear it is not too hard either).

A possible workaround is to specify add an explicit annotation get_actions: GetActions = ... in subclasses as well.

@delfick
Copy link
Author

delfick commented May 28, 2025

@ilevkivskyi right. That makes sense. Fair enough.

It appears get_actions: GetActions = ... works and there's only 33 places I have to do that in the repo I work on, so yeah, from the perspective of the 4 million lines of python I run mypy on, not worth blocking 1.16 over.

Thanks for taking a look!

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

5 participants