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

Possible regression with attrs.asdict related to type guarding? #1185

Closed
dougthor42 opened this issue Sep 20, 2023 · 7 comments
Closed

Possible regression with attrs.asdict related to type guarding? #1185

dougthor42 opened this issue Sep 20, 2023 · 7 comments

Comments

@dougthor42
Copy link

dougthor42 commented Sep 20, 2023

Possibly related issues:

Related PRs:

Summary

It looks like there's a regression with types guards and asdict from 21.4.0 to 23.1.0, and that they TypeGuard on attrs.has should be

def has(cls: type) -> TypeGuard[AttrsInstance]:

Or attrs.asdict should accept inst: Type[AttrsInstance]

Steps to Reproduce:

In 21.4.0, this worked:

# attrs-issue.py
from typing import Any
import attrs

@attrs.frozen
class Foo:
    a: int

obj: Any = Foo(0)

if attrs.has(obj):
    print(attrs.asdict(obj))
$ python attrs-issue.py 
{'a': 0}
$ mypy attrs-issue.py 
Success: no issues found in 1 source file

In 23.1.0, that same file fails mypy:

$ pip install attrs==23.1.0
Looking in indexes: https://pypi.org/simple
Collecting attrs==23.1.0
  Using cached attrs-23.1.0-py3-none-any.whl (61 kB)
Installing collected packages: attrs
  Attempting uninstall: attrs
    Found existing installation: attrs 21.4.0
    Uninstalling attrs-21.4.0:
      Successfully uninstalled attrs-21.4.0
Successfully installed attrs-23.1.0
$ python attrs-issue.py 
{'a': 0}
$ mypy attrs-issue.py 
attrs-issue.py:11: error: Argument 1 to "asdict" has incompatible type "type[AttrsInstance]"; expected "AttrsInstance"  [arg-type]
attrs-issue.py:11: note: ClassVar protocol member AttrsInstance.__attrs_attrs__ can never be matched by a class object
Found 1 error in 1 file (checked 1 source file)

Other Info

  • Python version: 3.10.10
  • Mypy version: 1.5.1 (compiled: yes)
  • Operating system: Debian GNU/Linux rodete, 64-bit
@Tinche
Copy link
Member

Tinche commented Sep 20, 2023

Looks like we have a regression but the type hints are correct. attrs.has is supposed to be called with a class, not an instance of a class, so your snippet should read if attrs.has(type(obj)):.

I think Mypy isn't clever enough to figure out that obj is an AttrsInstance if type(obj) is type[AttrsInstance]. Not sure what to do here.

@dougthor42
Copy link
Author

Ah, of course it's RTFM 🤣.

Looks like we have a regression

I guess it's just a regression in the sense that "incorrect code that happened to previously work no longer does". IMO that's less-bad than alternatives.

For the record, mypy is happy in both attrs versions when using attrs.has(type(obj)).

(Also, I noticed that the example code in the OP was missing a print(), so the issue was edited.)

@Tinche
Copy link
Member

Tinche commented Sep 20, 2023

attrs.has(type(obj)) didn't work for me, in the following block Mypy treated obj as Any. Which I guess is fine here, but not in the general case.

@hynek
Copy link
Member

hynek commented Dec 11, 2023

So uh, can this be closed?

@dougthor42
Copy link
Author

I'm fine with it, but given @Tinche's comment:

Looks like we have a regression but ...

I'll let them have the final say.

@hynek
Copy link
Member

hynek commented Dec 12, 2023

Tin, close if OK, otherwise pls expound next actions

@Tinche
Copy link
Member

Tinche commented Dec 13, 2023

I don't think we can do anything here, this seems like a Mypy limitation. We could start a discussion in the typing section of DPO. Closing this for now.

@Tinche Tinche closed this as completed Dec 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants