python / mypy

Optional static typing for Python
https://www.mypy-lang.org/
Other
18.13k stars 2.76k forks source link

at runtime dataclass special-cases FunctionType, but at type-time mypy special-cases Callable, leading to mismatches #14869

Open glyph opened 1 year ago

glyph commented 1 year ago

Bug Report

If you declare an attribute on a dataclass to be Callable, Mypy assumes that its __get__ will not be called, even on a default attribute. But this is only true if the default is a FunctionType, not some other type of callable.

Similarly, the inverse is true: an abstract type (such as a callable Protocol) will tell mypy that __get__ will be called, even though if the concrete default value is a FunctionType, it won't be.

Specifically, the following program contains 2 type errors, but MyPy does not think so:

To Reproduce

from __future__ import annotations
from typing import Callable, Protocol, ParamSpec, TypeVar, Generic
from dataclasses import dataclass

def f() -> None:
    print("hello world")

def f2(self: object) -> None:
    print("bound self", self)

P = ParamSpec("P")
T = TypeVar("T")
R = TypeVar("R", covariant=True)

T_con = TypeVar("T_con", contravariant=True)

class BindableMethod(Protocol[T_con, R]):
    def __get__(self, instance: T_con, owner: None | type[object]) -> Callable[[], R]:
        ...

    def __call__(me, self: T_con) -> R:
        ...

class UnboundNonFunction(Generic[P, R]):
    def __call__(self) -> int:
        print("unbound")
        return 3
    def __get__(self, instance: object, owner: type | None) -> BoundNonFunction:
        print("binding", instance, owner)
        return BoundNonFunction(instance)

@dataclass(frozen=True)
class BoundNonFunction:
    instance: object
    def __call__(self) -> str:
        print("bound", self.instance)
        return "wat"

@dataclass
class FuncHolder():
    func: Callable[[], None] = f
    func2: BindableMethod[FuncHolder, None] = f2
    func3: Callable[[], int] = UnboundNonFunction()

FuncHolder().func()
try:
    FuncHolder().func2()
except Exception as e:
    print("whoops", str(e))
try:
    print(FuncHolder().func3() + 4)
except Exception as e:
    print("whoops 2", str(e))

https://mypy-play.net/?mypy=latest&python=3.11&gist=1ac4e7c090774489207dfbc737de2d61

Expected Behavior

I would expect runtime and type-time behavior to be consistent here. Specifically I just don't expect __get__ to be called on things with a Callable type.

Actual Behavior

Mypy succeeds, rather than reporting the type errors.

Your Environment

zzzeek commented 1 year ago

I think for pep681 it's really important to keep pyright in the loop because the reference implementation is that of pyright: https://peps.python.org/pep-0681/#reference-implementation

for the above test case pyright only reports:

/home/classic/dev/sqlalchemy/test3.py /home/classic/dev/sqlalchemy/test3.py:21:18 - warning: Instance methods should take a "self" parameter (reportSelfClsParameterName)

for general pep-681 behavior you might want to raise it at https://github.com/python/peps/issues

ikonst commented 1 year ago

It's not that dataclasses discern between "Callable" and "FunctionType", it's just that anything that quacks like a descriptor triggers the behavior described here.

Note the __get__ is only called a couple times, with instance=None, meaning that it's not useful to e.g. bind a method to an instance. Maybe what you're hoping to do isn't even possible?

It looks to me like accidental behavior that got documented, given how it's a mere side-effect of how the default is represented (as a class variable's value), and how changing from Python syntax = d to = field(default=d) eliminates this behavior:

 class Descriptor:
     def __get__(self, instance, owner):
         return 42

 d = Descriptor()

 @dataclass 
class C:
-    desc: Descriptor = d
+    desc: Descriptor = field(default=d)

 c = C()
-assert c.desc == 42
+assert isinstance(c.desc, Descriptor)

Right now the plugin does not checks for field's type vs. default's type — it kinda happens on it own (due to typeshed annotations), thought it might be doable.

But is it even worthwhile to replicate given how odd and accidental this feature seems?