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

Revert "Add support for attrs.fields (#15021)" #15393

Closed
wants to merge 1 commit into from

Conversation

jhance
Copy link
Collaborator

@jhance jhance commented Jun 8, 2023

This reverts commit 391ed85.

The use of a Union item type for attr.Attribute would likely be more useful to callsites of attr.fields - otherwise this currently results in inferring builtins.object in many cases where we treat the fields as a variable-length tuple, rather than a fixed-length one. Applications of the fixed-length tuple seem limited since you would need to access it by index.

@Tinche as the original author of the commit.

This reverts commit 391ed85.

The use of a Union item type for `attr.Attribute` would likely be
more useful to callsites of `attr.fields` - otherwise this currently
results in inferring `builtins.object` in many cases where we treat
the fields as a variable-length tuple, rather than a fixed-length
one. Applications of the fixed-length tuple seem limited since you
would need to access it by index.

@Tinche as the original author of the commit.
Copy link
Collaborator

@JukkaL JukkaL left a comment

Choose a reason for hiding this comment

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

One option would be to infer a variable-length tuple with union item type. The issue here is that mypy doesn't use unions for joins.

@Tinche
Copy link
Contributor

Tinche commented Jun 8, 2023

@JukkaL I'm not in favor of this, see my comment over at #15021 (comment).

@github-actions
Copy link
Contributor

github-actions bot commented Jun 8, 2023

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

@ilevkivskyi
Copy link
Member

ilevkivskyi commented Jun 9, 2023

Limiting the tuple special-casing mentioned in #15396 (comment) to cases where fallback instance is builtins.tuple, or maybe better to when custom fallback doesn't override tuple.__iter__. But I think there may be a much simpler solution: have you tried making Attribute covariant (unless there is a good reason to keep it invariant)? IIRC mypy is now smart enough to infer Attribute[object] as iteration item for covariant generics.

@jhance
Copy link
Collaborator Author

jhance commented Jun 9, 2023

I don't think Attribute can be covariant because the validator object is clearly not. I looked into this yesterday.

hauntsaninja added a commit to hauntsaninja/mypy that referenced this pull request Jul 16, 2023
hauntsaninja added a commit to hauntsaninja/mypy that referenced this pull request Jul 16, 2023
@hauntsaninja
Copy link
Collaborator

Opened an alternate PR to fix this #15688

hauntsaninja added a commit to hauntsaninja/mypy that referenced this pull request Jul 16, 2023
@Tinche
Copy link
Contributor

Tinche commented Jul 29, 2023

@hauntsaninja thanks <3

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.

None yet

5 participants