Skip to content

attrs namedtuple #11794

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

Merged
merged 4 commits into from
Jan 7, 2022
Merged

attrs namedtuple #11794

merged 4 commits into from
Jan 7, 2022

Conversation

Tinche
Copy link
Contributor

@Tinche Tinche commented Dec 19, 2021

This PR further enhances the attrs plugin by generating a more correct __attrs_attrs__ field.

The current version of the plugin generates this field as a tuple of attributes. This enhancement makes it a namedtuple, which is what attrs actually does in runtime. The difference is a large increase in the ergonomics of the field: instead of A.__attrs_attrs__[0], a user can access an attribute using A.__attrs_attrs__.x. Also note the end-users don't actually use the __attrs_attrs__ field directly, but access it using attr.fields, which is a very simple getter interface.

This might sound like a trivial change, but it has big repercussions down the line for static checks in libraries building on top of attrs.

I have added 2 extra tests and improved an existing one.

@Tinche Tinche changed the title Tin/attrs namedtuple attrs namedtuple Dec 19, 2021
@Tinche
Copy link
Contributor Author

Tinche commented Dec 27, 2021

@sobolevn Hello, is there anything more I can do to nudge this along? (I tried the re-request review button, it did nothing.)

@sobolevn
Copy link
Member

@Tinche Sorry, I am not a real mypy developer, I am just helping with what I can 🙂
You need to ping someone with proper review permissions.

@Tinche
Copy link
Contributor Author

Tinche commented Dec 28, 2021

@euresti Hello, might I kindly ask for your feedback here?

@euresti
Copy link
Contributor

euresti commented Dec 29, 2021

@ilevkivskyi, can you give some advice on how to nudge this along?

Copy link
Member

@ilevkivskyi ilevkivskyi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TBH I didn't work on this for a while, but I don't see anything obviously wrong. I would however recommend adding a fine-grained daemon test, where you define a class, then update type of an attribute, and check that the change is propagated correctly to different file in fine-grained mode.

I will leave this for someone else to merge when the fine-grained test is added.

@Tinche
Copy link
Contributor Author

Tinche commented Jan 7, 2022

@ilevkivskyi thanks, added a fine-grained test. Hope I got it right!

@JelleZijlstra JelleZijlstra merged commit 254d41e into python:master Jan 7, 2022
@Tinche Tinche deleted the tin/attrs-namedtuple branch January 7, 2022 22:10
tushar-deepsource pushed a commit to DeepSourceCorp/mypy that referenced this pull request Jan 20, 2022
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

Successfully merging this pull request may close these issues.

5 participants