-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Fix FP for invalid-name with typing.Final on dataclass fields
#10797
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
base: main
Are you sure you want to change the base?
Fix FP for invalid-name with typing.Final on dataclass fields
#10797
Conversation
Codecov Reportβ
All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #10797 +/- ##
=======================================
Coverage 95.98% 95.99%
=======================================
Files 176 176
Lines 19564 19582 +18
=======================================
+ Hits 18779 18797 +18
Misses 785 785
π New features to boost your workflow:
|
|
π€ According to the primer, this change has no effect on the checked open source code. π€π This comment was generated for commit 4145bad |
Pierre-Sassoulas
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for taking so long to review but I feared my lack of typing practice was a problem and that Marc would have a better opinion about this.
| field_annotated_with_final: Final[int] = 42 | ||
| FIELD_ANNOTATED_WITH_FINAL: Final[int] = 42 # this could emit invalid-name eventually. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reading PEP591 I felt that Final was supposed to indicate a constant. Note that most of the Final annotated variable are in UPPERCASE here : https://peps.python.org/pep-0591/#id2 (with a notable exception of the mutable list and a 2 int vector example). Also here in the "updated documentation" linked in PEP 591 : https://docs.python.org/3/library/typing.html#typing.Final. Could you explain your reasoning for forbidding uppercase ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question. I can remove the editorial comment. I don't think there's any plan to forbid upper cased final variables.
I replied to the reporter, if it helps:
I'm a little surprised we enforce snake_case for function variables annotated with Final, since it seems like they should be UPPER_CASE, which would conflict with the example here, but I take it we should just be liberal and allow either.
Type of Changes
Description
Before, dataclass fields annotated with
Finalwere treated like all other class variables annotated withFinaland required to be uppercase. But that's not how dataclasses work -- you still want lower cased fields even if marked withFinal. An exception would beClassVar[Final], which doesn't give you a field.Closes #10790