-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
fix: update typevar handling when default is not set #7719
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
I believe the behaviour change in #7606 is raising false positives when the `default` argument is supported but not specified. Ref: https://peps.python.org/pep-0696/#implementation
|
please review |
| if default is None: | ||
| default = not_set |
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 the default not be None? How does this differentiate default=None from no default?
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.
According to the PEP:
- when default is unset,
__default__would beNone - if
default=None,__default__would beNoneType
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 we get rid of our internal unset thing then?
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.
Hmm, yes - let me try and see if that works
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.
Done :)
Change Summary
I believe the behaviour change in #7606 is raising false positives when the
defaultargument is supported but not specified.Ref: https://peps.python.org/pep-0696/#implementation
Related issue number
#7606
Checklist
Selected Reviewer: @Kludex