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

attrs: remove fields type check #15983

Merged
merged 1 commit into from Aug 30, 2023

Conversation

ikonst
Copy link
Contributor

@ikonst ikonst commented Aug 29, 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.

@ikonst ikonst force-pushed the attrs-remove-fields-type-check branch from 8d9cfc1 to 0bca9e9 Compare August 29, 2023 04:50
@github-actions
Copy link
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

prefect (https://github.com/PrefectHQ/prefect): typechecking got 1.10x slower (110.1s -> 120.5s)
(Performance measurements are based on a single noisy sample)

@ikonst
Copy link
Contributor Author

ikonst commented Aug 29, 2023

@sobolevn this is along the same lines as #15962

btw, the reason why I'm not culling the same code (wrote it at the same time...) for attrs.replace is that its signature is still attrs.replace(obj: T, ...) where T is not bound, i.e. the typeshed offers no validation of the first arg being an attrs instance.

Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

Thanks!

Copy link
Collaborator

@hauntsaninja hauntsaninja left a comment

Choose a reason for hiding this comment

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

Thanks for this!

@hauntsaninja hauntsaninja merged commit 2298829 into python:master Aug 30, 2023
18 checks passed
@ikonst ikonst deleted the attrs-remove-fields-type-check branch August 30, 2023 01:10
@Tinche
Copy link
Contributor

Tinche commented Nov 10, 2023

@ikonst haha thanks for this, I had it on my mental checklist

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.

Support for attr.has and attr.AttrsInstance
4 participants