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

Discrepancy in behavior of Callable instance fields #1113

Closed
JelleZijlstra opened this issue Mar 22, 2022 · 12 comments
Closed

Discrepancy in behavior of Callable instance fields #1113

JelleZijlstra opened this issue Mar 22, 2022 · 12 comments
Labels
topic: other Other topics not covered

Comments

@JelleZijlstra
Copy link
Member

(Expanding from python/typeshed#7520 (comment))

Consider this code:

from typing import Callable

class X:
    f: Callable[[], int]
    g: Callable[["X"], int]

reveal_type(X().f())
reveal_type(X().g())

Here is how current type checkers handle it:

  • pyright complains about the second call (Expected 1 more positional argument)
  • pytype complains about the second call (Function <callable> expects 1 arg(s), got 0 [wrong-arg-count])
  • pyanalyze complains about the second call (Missing required positional argument: '__arg0' (code: incompatible_call))
  • pyre accepts both calls (I think there's a bug where it doesn't complain about missing arguments?)
  • mypy complains about the first one (main.py:8: error: Attribute function "f" with type "Callable[[], int]" does not accept self argument).

I feel like mypy is wrong here, because the behavior where self gets implicitly bound on instances is specific to functions, not all callables. But the type system sometimes conflates functions with callables, so I can see where mypy is coming from.

In either case, the discrepancy is unfortunate because it means we can't write typeshed stubs that satisfy both type checkers. We should come to an agreement on the right behavior and document it.

@Akuli pointed out a workaround for mypy's behavior: using a callable protocol (python/typeshed#7520 (comment))

@gvanrossum
Copy link
Member

IIRC the mypy issue is because when you define a method it boils down to the same Callable[["X"], ...] type.

The problem is also with the conflation of class variables and instance variables in Python, where

class C:
    x = 0

could mean either an instance variable or a class variable.

@JukkaL
Copy link
Contributor

JukkaL commented Mar 22, 2022

Yeah, the mypy behavior is unfortunate. I bet this has been like this for a long time, probably from time before variable type annotations were a thing. I can have a look at how disruptive it would be to change this in mypy.

@erictraut
Copy link
Collaborator

erictraut commented Mar 22, 2022

I think pyright's behavior is correct here. It matches the runtime behavior (i.e. generates errors only in the situations where the runtime does).

from typing import Callable


def func1() -> int:
    return 0

def func2(self: "X") -> int:
    return 0

class X:
    f: Callable[[], int]
    g: Callable[["X"], int]

x = X()
x.f = func1 # mypy generates error on this line
x.g = func2 # mypy generates error on this line

x.f() # mypy generates error on this line
x.g() # pyright and Python runtime generate error on this line

could mean either an instance variable or a class variable

I don't think that distinction matters in this case. Here are two examples, one that populates the class variable and a second that populates an instance variable. In both cases, they must be called without passing a "self" argument.

This runs without error and type checks in both pyright and mypy.

from typing import Callable

def func1() -> int:
    return 0

class X:
    f: Callable[[], int]

X.f = func1
X.f()

This runs without error and type checks in pyright, but mypy generates errors.

from typing import Callable

def func1() -> int:
    return 0

class X:
    f: Callable[[], int]

x = X()
x.f = func1 # mypy: error: Attribute function "f" with type "Callable[[], int]" does not accept self argument
x.f() # mypy: error: Attribute function "f" with type "Callable[[], int]" does not accept self argument

@hauntsaninja
Copy link
Collaborator

hauntsaninja commented Mar 22, 2022

Yes, this is a long standing mypy bug: python/mypy#708

We had a fix recently (python/mypy#10548) but it had to be reverted in python/mypy#11571 because it regressed:

class C:
    def f(self) -> None: pass
    g = f

C().g()  # Sometimes generates an error

@JukkaL
Copy link
Contributor

JukkaL commented Mar 23, 2022

There may be a way to adjust python/mypy#10548 in a way that it is less disruptive to mypy users. For example, mypy could perhaps treat explicit annotations differently from inferred types from an assignment.

@glyph
Copy link

glyph commented Apr 1, 2022

I thought this bug had been fixed, because it definitely annoyed me a bit, and … it sort of has? But only if you're using dataclasses!

from typing import Callable
from dataclasses import dataclass

@dataclass
class X:
    f: Callable[[], int]
    g: Callable[["X"], int]

instance = X(lambda: 1, lambda self: 2)
reveal_type(instance.f())
reveal_type(instance.g(instance))

so is the bug here really just that some classes still aren't dataclasses for some weird reason? 😉

@glyph
Copy link

glyph commented Apr 1, 2022

More seriously the dataclasses/attrs plugin behavior is exactly what I'd expect, since f: Callable means a Callable instance variable; defining a method is ClassVar[Callable[... and implicitly assuming that any callable will bind self is still subtly wrong but a bit more justifiable than saying that callable instance vars become descriptors when they don't.

@gvanrossum
Copy link
Member

I agree that ideally it should work like that for all classes: require ClassVar to declare class variables. I expect that it may take mypy some effort to switch to this approach though. (Maybe @JukkaL can comment? Would it require a massive effort inside Dropbox to add ClassVar where needed?)

@JukkaL
Copy link
Contributor

JukkaL commented Apr 4, 2022

I believe that we can fix this without a huge migration for code that has been annotated using the existing semantics, but we may need some subtle adjustments to the rules around class/instance variables. I'll investigate this in more detail, hopefully in the next one or two weeks.

@JelleZijlstra
Copy link
Member Author

Closing this because we're in agreement that it's a bug in mypy. Thanks for the insights!

@cjw296
Copy link

cjw296 commented Sep 9, 2023

@JelleZijlstra - where's the mypy bug tracking this? python/mypy#708 is closed but the problem is still there on Python 3.11 and latest mypy. (see simplistix/sybil#78 (comment))

@JelleZijlstra
Copy link
Member Author

Not sure, there might not be one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: other Other topics not covered
Projects
None yet
Development

No branches or pull requests

7 participants