mypy icon indicating copy to clipboard operation
mypy copied to clipboard

Attrs/tweak magic attribute

Open Tinche opened this issue 3 years ago • 7 comments

This is an attempt to fix the attrs magic attribute handling.

The attrs magic attribute functionality was added by me a few months ago, but inexpertly. Since we're starting to see usage in the wild, I'm attempting to fix it.

Context: every attrs class has a magic attribute, a final classvar available under Class.__attrs_attrs__. This is an instance of an ad-hoc tuple subclass, and it contains an instance of attrs.Attribute[T] for each class attribute. The reason it's a tuple subclass is that is has properties for each attribute, so instead of Class.__attrs_attrs__[0] we can use Class.__attrs_attrs__.a. Users aren't really supposed to use this attribute directly, but through attrs.fields() (a subject of a future PR), but having Mypy know about it is useful for f.e. protocols (and the newest release of attrs does contain a protocol using the magic attribute).

When I implemented the magic attribute here initially, I set it as no_serialize=True. This is incorrect for some use cases.

My first issue: is my approach OK?

Like I mentioned, the type of each magic attribute is an instance of an ad-hoc tuple subclass. I need to stash this subclass somewhere so cached runs work properly. Right now I'm trying to serialize it under Class.__Class_AttrsAttributes__; maybe there's a better way? I needed to incorporate the fully qualified class name in the attribute name so Mypy doesn't complain about incompatible fields when subclassing.

My second issue is some incremental, fine-grained tests are failing now and I cannot figure out how to fix them, so it'd be great to get some advice here too.

Tinche avatar Aug 26 '22 13:08 Tinche

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

github-actions[bot] avatar Aug 26 '22 13:08 github-actions[bot]

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

github-actions[bot] avatar Aug 29 '22 02:08 github-actions[bot]

Alright, I've fixed the tests so everything should be gucci on the test matrix.

This is ready for a design and implementation review.

Tinche avatar Aug 29 '22 13:08 Tinche

@sobolevn After some context at the last typing meetup, any chance of a review here? ;)

Tinche avatar Sep 14 '22 17:09 Tinche

Won't it be easier for you to ship your own plugin? While mypy can have only basic support of attrs for regular use-cases.

You're absolutely right, the goal is in the long term to have this plugin as part of attrs, and architecturally it's the right call. I fear if we did this now though it would effectively kill the plugin since we're still building our Mypy expertise. By having it in Mypy we can get feedback from Mypy devs like you who are much more knowledgeable about the internals of Mypy than we are, and we can count on you folks to keep the plugin working because our tests are here.

Obviously this is a burden on Mypy maintainers so whenever you folks want us out, we'll get out. But for now I'd like to stay in ;)

As for the other PR feedback, I'll take a look soon.

Tinche avatar Sep 15 '22 16:09 Tinche

Obviously this is a burden on Mypy maintainers so whenever you folks want us out

We totally don't 😉

sobolevn avatar Sep 15 '22 19:09 sobolevn

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

github-actions[bot] avatar Sep 22 '22 00:09 github-actions[bot]

🤖 ✅

sobolevn avatar Sep 26 '22 06:09 sobolevn