Add __dataclass_fields and attrs_attrs__ to dataclasses #8578

tkukushkin · 2020-03-23T14:28:16Z

tkukushkin · 2020-03-23T15:26:26Z

mypy/plugins/attrs.py

+def _add_attrs_magic_attribute(ctx: 'mypy.plugin.ClassDefContext') -> None:
+    attr_name = '__attrs_attrs__'
+    attr_type = ctx.api.named_type('__builtins__.tuple', [
+        ctx.api.named_type('attr.Attribute'),


@ilevkivskyi ctx.api.named_type is for builtin types, but what i should use instead of it?

It is not for builtin types, it is for types that are guaranteed to be present (i.e. loaded).

@ilevkivskyi, and how to ensure that attr.Attribute is loaded? Or maybe need to use Any if it is not loaded?

They should be loaded, because the attr module must be analyzed if this plugin is called. The tests fail because they run with partial stubs, not the real ones, see for example https://github.com/python/mypy/blob/master/test-data/unit/lib-stub/attr.pyi, Attribute is not present there.

Also the latter is generic, so I would explicitly add a type argument here (even if it is Any).

They should be loaded, because the attr module must be analyzed if this plugin is called.

I have a problem with different styles of import of attr.

This example works fine:

import attr @attr.s class Foo: pass class AttrsProtocol(Protocol): __attrs_attrs__: Tuple[attr.Attribute, ...] def foo(x: AttrsProtocol) -> None: pass foo(Foo())

But if I change import to:

from attr import attrs, Attribute @attrs class Foo: pass

I get exception:

File "/Users/tkukushkin/Projects/mypy/mypy/plugins/attrs.py", line 664, in _add_attrs_magic_attribute ctx.api.named_type('attr.Attribute'), File "/Users/tkukushkin/Projects/mypy/mypy/semanal.py", line 4143, in named_type assert sym, "Internal error: attempted to construct unknown type" AssertionError: Internal error: attempted to construct unknown type

Some tests fail because of this.

The tests fail because they run with partial stubs, not the real ones, see for example https://github.com/python/mypy/blob/master/test-data/unit/lib-stub/attr.pyi, Attribute is not present there.

Yes, I already added Attribute to stub, but not pushed yet.

Well, I just looked at the code of named_type() and it is horrible, it uses lookup_qualified() (which is just a normal symbol lookup following Python name resolution rules) so I wonder how it ever worked. You can try using named_type_or_none() instead, that one looks reasonable.

Another option is to actually fix named_type(), but it may be not the best task as a first contribution.

ilevkivskyi

Please add few dedicated tests and make this a non-draft.

ilevkivskyi · 2020-03-26T22:17:50Z

mypy/plugins/attrs.py

+def _add_attrs_magic_attribute(ctx: 'mypy.plugin.ClassDefContext') -> None:
+    attr_name = '__attrs_attrs__'
+    attr_type = ctx.api.named_type('__builtins__.tuple', [
+        ctx.api.named_type('attr.Attribute'),


They should be loaded, because the attr module must be analyzed if this plugin is called. The tests fail because they run with partial stubs, not the real ones, see for example https://github.com/python/mypy/blob/master/test-data/unit/lib-stub/attr.pyi, Attribute is not present there.

Also the latter is generic, so I would explicitly add a type argument here (even if it is Any).

ilevkivskyi · 2020-03-26T22:18:12Z

mypy/plugins/dataclasses.py

@@ -361,6 +363,21 @@ def _freeze(self, attributes: List[DataclassAttribute]) -> None:
                var._fullname = info.fullname + '.' + var.name
                info.names[var.name] = SymbolTableNode(MDEF, var)

+    def _add_dataclass_fields_magic_attribute(self) -> None:
+        attr_name = '__dataclass_fields__'
+        dataclass_field_type = self._ctx.api.named_type('dataclasses.Field')


Same comment here.

…bute

tkukushkin · 2020-03-29T15:49:56Z

@ilevkivskyi Thanks for your suggestions. named_type_or_none works fine, but ClassDefContext.api has type SemanticAnalyzerPluginInterface which hasn't member named_type_or_none. Should I add this method to interface?

And second question: how do you recommend to fix tests testNarrowingParentWithEnumsBasic and testNarrowingParentWithIsInstanceBasic. They use fixtures/tuple.pyi and fixtures/isinstance.pyi respectively. These partial stubs don't contain dict, which I use in _add_dataclass_fields_magic_attribute. Should I create special partial stubs for this tests, or split these tests on several small tests with different objects, or extend existing stubs by simple dict definition (class dict(Generic[_KT, _VT]): pass)?

test-data/unit/check-dataclasses.test

ilevkivskyi · 2020-04-05T18:07:13Z

@tkukushkin Thanks for updates! Here are answers to your questions:

Should I add this method to interface?

I think it would make sense to add it.

Should I create special partial stubs for this tests, or split these tests on several small tests with different objects, or extend existing stubs by simple dict definition

I think creating dedicated partial stubs (fixtures) is fine.

maybe this test is not valid.

Good catch! Yeah, I think this test passes because of bad partial stubs. In real stubs object always has __eq__() so the code you linked is wrong, it should read info.names.get('__eq__') is None instead of info.get('__eq__') is None (to avoid MRO lookup and only check current class).

You can open a separate issue/PR for this.

tkukushkin · 2020-04-09T10:02:48Z

Fixed __eq__ in #8642

tkukushkin · 2020-10-22T15:19:06Z

@JukkaL maybe you can review this pr?

JelleZijlstra

A few small comments, and CI is currently failing.

mypy/plugin.py

mypy/plugins/attrs.py

test-data/unit/foo.py

JelleZijlstra

Looks good apart from the stray file

JelleZijlstra

Thanks! I'll merge once CI passes (if I don't, feel free to remind me).

eviltwin · 2021-09-21T10:23:47Z

I really don't want to be "that guy" and ask when this will see a release... but do you have published release cadences? This has been merged for a month now and the last release was in June. I'm not trying to phrase this in an "omg what's the hold up?" kind of way - I totally get that other things can be blocking the release.

It's just that I happened across an issue this morning whilst consuming djangorestframework-dataclasses that would be solved by this feature, so I'm a little itchy to see it released :)

JukkaL · 2021-09-21T18:00:45Z

@eviltwin Tracking issue for the next release: #11158

Tinche · 2021-12-16T11:38:54Z

@tkukushkin Thanks a lot for this, I think this work is going to be very valuable going forward and I can close my failed attempt at #10467. How difficult would it be to have attr.fields(A) actually return A.__attrs_attrs__? That API is much nicer than the dunder (although the dunder should stay for protocol checking etc).

tkukushkin · 2021-12-17T09:20:28Z

@Tinche I spent some time and got such a proof of concept:

class AttrsFieldsPlugin(Plugin):

    def get_function_signature_hook(self, fullname: str) -> Optional[Callable[[FunctionSigContext], FunctionLike]]:
        if fullname == 'attr.fields':
            return self._attrs_fiels_hook
        return None

    def _attrs_fiels_hook(self, context: FunctionSigContext) -> FunctionLike:
        return CallableType(
            arg_types=[AnyType(TypeOfAny.explicit)],
            arg_kinds=[ArgKind.ARG_POS],
            arg_names=['cls'],
            ret_type=context.args[0][0].node.names['__attrs_attrs__'].type,
            fallback=context.api.named_type('builtins.function'),
        )

Unfortunately, I don't have time to finalize this solution (need to correct the type of the function argument and deal with the situation when not attrs class passed to the attrs.fields)

I also understand attrs.fields returns named tuple, in this solution the function returns regular tuple, and I'm not sure if there is a way to fix that.

Tinche · 2021-12-17T11:57:12Z

@tkukushkin

Hello again! Thanks for looking into this.

I think fields doesn't need to be handled by the plugin at all. I've created a PR for attrs adding a protocol and making fields work out of the box: python-attrs/attrs#890

The problem isn't fields, but as you said the fact the plugin generates tuples instead of namedtuples for __attrs_attrs__. So the Mypy plugin needs to be improved to generate a namedtuple subclass for each attrs class it sees and assign that to __attrs_attrs__. I will attempt this but since I'm not super knowledgeable about Mypy would appreciate pointers ;)

In any case, thanks for getting us this far.

tkukushkin · 2021-12-17T12:09:36Z

@Tinche I looked at your pull request – beautiful solution, didn't think of that. But yes, I don't really know how to make __attrs_attrs__ become namedtuple :-(.

Blocking issue resolved by this update to mypy python/mypy#6568 python/mypy#8578 we can now check that a DataClass object conforms to our ConfigProtocol Protocol, allowing us to pass ally mypy checks.

tkukushkin added 2 commits March 23, 2020 17:23

Add __attrs_attrs__ attribute to attr.s decorated classes

07f8894

Add __dataclass_fields__ attribute to dataclasses

abfe951

tkukushkin commented Mar 23, 2020

View reviewed changes

ilevkivskyi reviewed Mar 26, 2020

View reviewed changes

tkukushkin added 4 commits March 27, 2020 08:27

Add explicit Any as type argument to dataclasses.Field and attr.Attri…

4a8688e

…bute

Fix some tests

17a9403

Fix almost all tests

229c568

Remove commented code from stub

426d644

tkukushkin commented Mar 29, 2020

View reviewed changes

test-data/unit/check-dataclasses.test Outdated Show resolved Hide resolved

ilevkivskyi mentioned this pull request May 11, 2020

Recognizing wrapped classes as supporting a Protocol #6568

Closed

tkukushkin added 5 commits October 11, 2020 14:00

Merge branch 'master' into fix-6568

0b97db1

Fix narrowing tests

8dadb8a

Fix test

62b8497

Fix mypy errors

7b26c67

Fix flake8 errors

a5bcda1

tkukushkin marked this pull request as ready for review October 11, 2020 12:44

tkukushkin requested a review from ilevkivskyi October 13, 2020 07:12

Merge branch 'master' into fix-6568

e75b83a

JelleZijlstra requested changes Jan 16, 2021

View reviewed changes

mypy/plugin.py Outdated Show resolved Hide resolved

mypy/plugins/attrs.py Outdated Show resolved Hide resolved

Fix tests

0932037

tkukushkin requested a review from JelleZijlstra January 23, 2021 16:52

tkukushkin added 4 commits August 13, 2021 10:19

Merge branch 'master' of github.com:python/mypy into fix-6568

3a66912

Generate precise type for __attrs_attrs__ in attrs plugin

2d3a6ca

Fix self mypy check

e932c50

Fix tests

be97c26

JelleZijlstra reviewed Aug 20, 2021

View reviewed changes

test-data/unit/foo.py Outdated Show resolved Hide resolved

JelleZijlstra requested changes Aug 20, 2021

View reviewed changes

Delete stray file

528d587

JelleZijlstra approved these changes Aug 20, 2021

View reviewed changes

JelleZijlstra merged commit 7576f65 into python:master Aug 20, 2021

ruancomelli mentioned this pull request Aug 24, 2021

Support for Protocols beartype/plum#28

Closed

freundTech added a commit to freundTech/mypy that referenced this pull request Sep 27, 2021

Fix failing tests after merge (Caused by python#8578)

f2e12f8

Tinche mentioned this pull request Dec 16, 2021

Add __attrs_attrs__ and fields to attrs #10467

Closed

tkukushkin deleted the fix-6568 branch December 17, 2021 08:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add __dataclass_fields and attrs_attrs__ to dataclasses #8578

Add __dataclass_fields and attrs_attrs__ to dataclasses #8578

tkukushkin commented Mar 23, 2020

tkukushkin Mar 23, 2020

ilevkivskyi Mar 23, 2020

tkukushkin Mar 24, 2020

ilevkivskyi Mar 26, 2020

tkukushkin Mar 27, 2020

ilevkivskyi Mar 27, 2020

ilevkivskyi left a comment

ilevkivskyi Mar 26, 2020

ilevkivskyi Mar 26, 2020

tkukushkin commented Mar 29, 2020 •

edited

Loading

ilevkivskyi commented Apr 5, 2020

tkukushkin commented Apr 9, 2020

tkukushkin commented Oct 22, 2020

JelleZijlstra left a comment

JelleZijlstra left a comment

JelleZijlstra left a comment

eviltwin commented Sep 21, 2021

JukkaL commented Sep 21, 2021

Tinche commented Dec 16, 2021

tkukushkin commented Dec 17, 2021

Tinche commented Dec 17, 2021

tkukushkin commented Dec 17, 2021

Add __dataclass_fields__ and __attrs_attrs__ to dataclasses #8578

Add __dataclass_fields__ and __attrs_attrs__ to dataclasses #8578

Conversation

tkukushkin commented Mar 23, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ilevkivskyi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tkukushkin commented Mar 29, 2020 • edited Loading

ilevkivskyi commented Apr 5, 2020

tkukushkin commented Apr 9, 2020

tkukushkin commented Oct 22, 2020

JelleZijlstra left a comment

Choose a reason for hiding this comment

JelleZijlstra left a comment

Choose a reason for hiding this comment

JelleZijlstra left a comment

Choose a reason for hiding this comment

eviltwin commented Sep 21, 2021

JukkaL commented Sep 21, 2021

Tinche commented Dec 16, 2021

tkukushkin commented Dec 17, 2021

Tinche commented Dec 17, 2021

tkukushkin commented Dec 17, 2021

Add __dataclass_fields and attrs_attrs__ to dataclasses #8578

Add __dataclass_fields and attrs_attrs__ to dataclasses #8578

tkukushkin commented Mar 29, 2020 •

edited

Loading