-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Do not allow to use Final and ClassVar at same time #10478
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
Conversation
from typing import Final, ClassVar | ||
|
||
class A: | ||
a: Final[ClassVar[int]] # E: Variable should not be annotated with both ClassVar and Final |
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.
Can you test some other variants, such as a: ClassVar[Final] = 1
, a: ClassVar[Final[int]]
?
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.
Such cases will produce an error with message:
Final can be only used as an outermost qualifier in a variable annotation.
Should it be changed to Variable should not be annotated with both ClassVar and Final
?
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.
If it's easy to do, then yes. The existing error message would make users first swap the qualifiers, then get an error anyway from your new check, which is not very user-friendly. But don't worry about changing the error message if it's complicated.
Either way I think it would be useful to test all these variations here. They can all go in the same test case.
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.
I have added tests that you mention, thanks for pointing this out.
Looks like it is not easy to change message for a ClassVar[Final[int]]
case.
I will skip this for this moment.
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.
Looks good, but I'm going to hold off on merging because GitHub Actions are broken at the moment.
This PR allows to use Final and ClassVar after python 3.13 I saw this [PR](#10478) and I saw recent changes of python 3.13 https://docs.python.org/3/library/typing.html#typing.Final Final now can be nested with ClassVar. so I added a version check! --------- Co-authored-by: triumph1 <seungwon.jeong@wesang.com> Co-authored-by: hauntsaninja <hauntsaninja@gmail.com>
Description
Fixes #10477
Do not allow to use Final and ClassVar at same time.
Test Plan
Code bellow:
After this PR will produce an error:
But currently, it will not raise any error.