Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Mypy improvements #890

Merged
merged 7 commits into from
May 1, 2022
Merged

Mypy improvements #890

merged 7 commits into from
May 1, 2022

Conversation

Tinche
Copy link
Member

@Tinche Tinche commented Dec 17, 2021

Not to be merged yet.

The problem is the Mypy plugin doesn't return a namedtuple, but an ordinary tuple. We should fix this in the Mypy plugin.

@hynek
Copy link
Member

hynek commented Feb 2, 2022

Is this something that should remain open?

@Tinche
Copy link
Member Author

Tinche commented Feb 2, 2022

Yeah, we need to merge this when Mypy release 0.940 I believe.

@hynek
Copy link
Member

hynek commented Feb 2, 2022

Maybe we should get it to pass CI until then. 😇

@Tinche
Copy link
Member Author

Tinche commented Feb 2, 2022

I can't until Mypy releases :p

@hynek
Copy link
Member

hynek commented Feb 2, 2022

eg ok, your phrasing sounded like it's needed, once mypy is out

@Tinche
Copy link
Member Author

Tinche commented Feb 2, 2022

It's just more sophisticated type stubs, we don't have to match their release exactly. But we might want to.

@hkclark
Copy link
Contributor

hkclark commented Feb 2, 2022

@Tinche I noticed recently that using attrs 21.4.0 and the new attrs namespace didn't seem to be supported by mypy. I was initially thinking this PR might add it, but in taking a quick look at the changes, it doesn't seem so (but I'm not expert on mypy type syntax). I was just curious if there was a plan to get support for the attrs namespace or if I was just doing something wrong on my end. :-) Thanks for your work on this!

@hynek
Copy link
Member

hynek commented Feb 2, 2022

I noticed recently that using attrs 21.4.0 and the new attrs namespace didn't seem to be supported by mypy.

The new attrs namespace is absolutely supported by mypy. I use it all the time.

We've achieved it by re-importing symbols from the attr namespace and mypy can resolve that.

@hkclark
Copy link
Contributor

hkclark commented Feb 2, 2022

I noticed recently that using attrs 21.4.0 and the new attrs namespace didn't seem to be supported by mypy.

The new attrs namespace is absolutely supported by mypy. I use it all the time.

We've achieved it by re-importing symbols from the attr namespace and mypy can resolve that.

Hmm, OK, thanks for the info! I guess let me convert this project at work back to the attrs namespace and see if I can figure out what's going on. :-)

While I have you, I assume it's expected that mypy cannot handle "fancy converters" -- I get error: Unsupported converter, only named functions and types are currently supported if I do things like: a) converter=lambda value: weakref.proxy(value) or converter=[...] or converter=attrs.validtors.pipe(...).

@hkclark
Copy link
Contributor

hkclark commented Feb 2, 2022

@hynek To follow up on the issues I mentioned in this project I'm working on, when I use the attrs namespace for everything I get this from mypy for my converters:

error: Module has no attribute "converters"  [attr-defined]

@hynek
Copy link
Member

hynek commented Feb 3, 2022

Unfortunately, mypy support for converters is very bare-bones. There's not much we can do about it. Your error must have some other cause. It's explicitly tested here:

from attrs.converters import optional

@hkclark
Copy link
Contributor

hkclark commented Feb 3, 2022

Unfortunately, mypy support for converters is very bare-bones. There's not much we can do about it. Your error must have some other cause. It's explicitly tested here:

from attrs.converters import optional

@hynek OK, thank you. I'll study it more and if I can't find a cause on my side, I'll try to open a ticket with very bare-bones example code that recreates it. And maybe I'll open a PR that adds a comment to the docs re converters being limited just so others know about the limitations. Thank you again!

@Tinche
Copy link
Member Author

Tinche commented Mar 14, 2022

@hynek this should go in now. I guess I need to rebase though.

@hynek
Copy link
Member

hynek commented Mar 14, 2022

@hkclark turns out you were right and our tests broken: #930 + #931

Sorry for that!

@hynek
Copy link
Member

hynek commented Mar 14, 2022

@Tinche this is breaking even on 3.6…wanna wait until this stuff is fixed in mypy? :-/

@hkclark
Copy link
Contributor

hkclark commented Mar 14, 2022

@hkclark turns out you were right and our tests broken: #930 + #931

Sorry for that!

@hynek No problem at all. Thanks for getting back to me. I'm just glad to know I wasn't going totally crazy. :-) Thank you again for your work on attrs... I really like it and look forward to ongoing new features.

@Tinche
Copy link
Member Author

Tinche commented Mar 16, 2022

Mergy mergy now?

@hynek
Copy link
Member

hynek commented Mar 16, 2022

Allmost all of CI redy redy?

@hynek
Copy link
Member

hynek commented Apr 30, 2022

  • why does test_mypy hate us, personally?
  • it should be something like "AttrsClassInstance" or "AttrsInstance" for all I care?

@hynek hynek merged commit c4ded79 into main May 1, 2022
@hynek hynek deleted the tin/mypy-fields branch May 1, 2022 16:43
hauntsaninja pushed a commit to python/mypy that referenced this pull request Aug 30, 2023
Since python-attrs/attrs#890 (≥ 22.1.0)
`attrs.fields` is typed to accept a protocol.
Since python-attrs/attrs#997 (≥ 22.2.0)
`attrs.has` is a type-guard.

Support both by removing the explicit error reporting and letting it
fall through to the type stub.

Fixes #15980.
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants