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

Dataclass "init" method should expect the type from a custom descriptor's "set" method #13856

Closed
pappasam opened this issue Oct 10, 2022 · 5 comments
Labels
bug mypy got something wrong topic-dataclasses topic-descriptors Properties, class vs. instance attributes topic-runtime-semantics mypy doesn't model runtime semantics correctly

Comments

@pappasam
Copy link

This issue is copied, mostly verbatim, from here: microsoft/pyright#3245.

If a @dataclass field is initialized with a descriptor as a default value, mypy should use the value type of the __set__ method for the descriptor when determining the type of the corresponding parameter within the synthesized __init__ method.

Types are also evaluated incorrectly in this case.

from dataclasses import dataclass
from typing import Any, cast


class MyDescriptor:
    def __get__(self, __obj: object | None, __owner: Any) -> "int | MyDescriptor":
        if __obj is None:
            return self
        return cast(Any, __obj)._x

    def __set__(self, __obj: object | None, __value: int) -> None:
        if __obj is not None:
            cast(Any, __obj)._x = __value


@dataclass
class Foo:
    y: MyDescriptor = MyDescriptor()


f = Foo(3)  # Currently a false positive error in mypy

print(f.y)  # 3
f.y = 4
print(f.y)  # 4
print(Foo.y)  # MyDescriptor object

reveal_type(f.y) # int | MyDescriptor, which is wrong
reveal_type(Foo.y) # int | MyDescriptor, which is wrong

To Reproduce

Run mypy against the above script.

Expected Behavior

No Errors

Actual Behavior

test.py:23: error: Argument 1 to "Foo" has incompatible type "int"; expected "MyDescriptor"
test.py:30: note: Revealed type is "Union[builtins.int, test.MyDescriptor]"
test.py:31: note: Revealed type is "Union[builtins.int, test.MyDescriptor]"
Found 1 error in 1 file (checked 1 source file)

Your Environment

  • OS: Ubuntu 22.04.1
  • Mypy version used: 0.982
  • Mypy command-line flags: None
  • Mypy configuration options from mypy.ini (and other config files): None
  • Python version used: 3.10.6
@pappasam pappasam added the bug mypy got something wrong label Oct 10, 2022
@pappasam
Copy link
Author

@erictraut fyi, I migrated your issue from pyright here

@debonte
Copy link

debonte commented Mar 10, 2023

FYI, the behavior of dataclass with descriptor-typed fields was discussed in python/cpython#91330 and is documented here.

@AlexWaygood AlexWaygood added topic-dataclasses topic-descriptors Properties, class vs. instance attributes topic-runtime-semantics mypy doesn't model runtime semantics correctly labels Mar 10, 2023
@glyph
Copy link

glyph commented Mar 10, 2023

I think this confusion goes a bit deeper than __set__; Mypy seems to have the wrong idea about dataclass-like types in general non-dataclass PEP 681 classes such as attrs. Consider this brief comparison of attrs and dataclasses:

from __future__ import annotations
from typing import reveal_type, TYPE_CHECKING
from attr import define

class DecoGet:
    pass

class Deco:
    def __set__(self, oself: object, value: Deco) -> None:
        ...
    def __get__(self, oself: object, owner: None | type[object]=None) -> DecoGet:
        return DecoGet()

@define
class AttrBlub:
    a: Deco
    b: Deco = Deco()


from dataclasses import dataclass

@dataclass
class DataBlub:
    a: Deco
    b: Deco = Deco()

reveal_type(AttrBlub(Deco()).a)
reveal_type(AttrBlub(Deco()).b)

reveal_type(DataBlub(Deco()).a)
reveal_type(DataBlub(Deco()).b)

reveal_type(AttrBlub.a)
reveal_type(AttrBlub.b)

try:
    reveal_type(DataBlub.a)     # AttributeError at runtime
except:
    print("skipped")
reveal_type(DataBlub.b)

Side-by-side, type-check and output looks like this (emojis added to indicate agreement / disagreement between type-time and runtime):

t.py:27: note: Revealed type is "t.DecoGet"    Runtime type is 'Deco'              ❌ attrs / instance / no default
t.py:28: note: Revealed type is "t.DecoGet"    Runtime type is 'Deco'              ❌ attrs / instance / default
t.py:30: note: Revealed type is "t.Deco"       Runtime type is 'Deco'              ✅ dataclasses / instance / no default
t.py:31: note: Revealed type is "t.DecoGet"    Runtime type is 'DecoGet'           ✅ dataclasses / instance / default
t.py:33: note: Revealed type is "t.DecoGet"    Runtime type is 'member_descriptor' ❌ attrs / class / no default
t.py:34: note: Revealed type is "t.DecoGet"    Runtime type is 'member_descriptor' ❌ attrs / class / default
t.py:37: note: Revealed type is "t.DecoGet"    skipped                             ❌ dataclasses / class / no default
t.py:40: note: Revealed type is "t.DecoGet"    Runtime type is 'DecoGet'           ✅ dataclasses / class / default

Updated to include annotations about which one's which, which shows that the problem is with attrs' annotations more than it is with dataclasses, although "attribute error" vs. descriptor-type is not great.

@glyph
Copy link

glyph commented Mar 10, 2023

As I was playing with this I discovered a more specific problem in this neighborhood with Callable #14869

sqlalchemy-bot pushed a commit to sqlalchemy/sqlalchemy that referenced this issue Mar 12, 2023
Mypy 1.1.1 has been released which includes a non-compliant pep-681
implementation that fails with SQLAlchemy's :class:`.MappedAsDataclass` and
similar features. In order to work around this issue until Mypy is able to
release a fix, as well as to support other typing tools which may have
non-compliant pep-681 implementations, document a workaround class
for :class:`.MappedAsDataclass`.

Including this class as well as a decorator was considered, but overall
this is an issue with typing tools that they will have to resolve
and I'm not ready to set up for this issue going on long term.  There's
also no good solution for the decorator version since you have to
have an ``__init__`` method indicated somewhere.

References: python/mypy#13856
Fixes: #9467
Change-Id: I1be6abea7f7fc72883c14ab2447edad937d0c23f
matusvalo pushed a commit to matusvalo/sqlalchemy that referenced this issue Mar 13, 2023
Mypy 1.1.1 has been released which includes a non-compliant pep-681
implementation that fails with SQLAlchemy's :class:`.MappedAsDataclass` and
similar features. In order to work around this issue until Mypy is able to
release a fix, as well as to support other typing tools which may have
non-compliant pep-681 implementations, document a workaround class
for :class:`.MappedAsDataclass`.

Including this class as well as a decorator was considered, but overall
this is an issue with typing tools that they will have to resolve
and I'm not ready to set up for this issue going on long term.  There's
also no good solution for the decorator version since you have to
have an ``__init__`` method indicated somewhere.

References: python/mypy#13856
Fixes: sqlalchemy#9467
Change-Id: I1be6abea7f7fc72883c14ab2447edad937d0c23f
maartenbreddels added a commit to widgetti/solara that referenced this issue Mar 24, 2023
This already works with pyright due to:
    microsoft/pyright#3245

but mypy does not yet support this:
    python/mypy#13856

See also:
    python/mypy#14868
maartenbreddels added a commit to widgetti/solara that referenced this issue Mar 24, 2023
This already works with pyright due to:
    microsoft/pyright#3245

but mypy does not yet support this:
    python/mypy#13856

See also:
    python/mypy#14868
@pappasam
Copy link
Author

Just tested out with mypy 1.2 and this appears to have been resolved!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mypy got something wrong topic-dataclasses topic-descriptors Properties, class vs. instance attributes topic-runtime-semantics mypy doesn't model runtime semantics correctly
Projects
None yet
Development

No branches or pull requests

4 participants