-
-
Notifications
You must be signed in to change notification settings - Fork 30.5k
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
gh-103556: [inspect.Signature] disallow pos-or-kw params without default after pos-only with default #103557
Conversation
…t default after pos-only with default
|
@hauntsaninja if you are interested in this as well after #103554 I would be very grateful :) |
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.
Thanks for finding and fixing this!
I think the fix can be even simpler, though; see inline comment.
Lib/inspect.py
Outdated
if (top_kind != _POSITIONAL_ONLY | ||
and kind != _POSITIONAL_OR_KEYWORD): | ||
# We still have to maintain defaults in cases like | ||
# def some(pod=42, /, pk=1): ... | ||
# Here `pk` must have a default value. | ||
kind_defaults = False |
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.
non-default-after-default is only checked for _POSITIONAL_ONLY
and _POSITIONAL_KEYWORD
(see line 3032.) So if we want to keep the value of kind_defaults
across the transition from PO to POK (which I do agree is correct and matches the parser behavior), then really we always want to keep it, and it is no longer kind_defaults
but simply seen_default
. So I think we can eliminate all of this, here's the full version I'd recommend (which passes all tests): https://gist.github.com/carljm/6779e601d04e0b1135444b4e0263e281
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.
Yes, I agree, thank you for the suggestion.
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
I have made the requested changes; please review again |
Thanks for making the requested changes! @carljm: please review the changes made to this pull request. |
…to issue-103556
…t default after pos-only with default (pythonGH-103557) (cherry picked from commit 6b58d41) Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
GH-103675 is a backport of this pull request to the 3.11 branch. |
I consider this a bug fix, not a new feature. So, backport to 3.11 should probably be done.
inspect.Signature
can be created: pos-only with a default followed by pos-or-kw without one #103556