-
-
Notifications
You must be signed in to change notification settings - Fork 33k
gh-137317: fix inspect signature of class with descriptor wrapper #137862
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
Signed-off-by: GitHub <[email protected]>
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
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.
Thank you for your PR, @yanyongyu. LGTM. 👍
I only have few minor suggestions for tests.
|
||
with self.subTest('partial'): | ||
class C: | ||
__init__ = identity(functools.partial(lambda x, a, b: None, 2)) |
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.
Should not this be functools.partial(identity(lambda x, a, b: None), 2)
? Although the "partial" and "partialmethod" subtests are not affected by all these changes, so I am not sure that it makes sense to keep them. If they do not fail, we cannot be sure that they are meaningful.
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.
Thank you for pointing out the error, I will correct this.
These test cases are based on the ones below (test_signature_on_class_with_init
and test_signature_on_class_with_new
), and I believe they need to be this detailed to catch potential bugs from changes made in inspect.signature
's behavior.
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.
LGTM. 👍
I added also tests for a wrapped metaclass' |
Thanks @yanyongyu for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14. |
Thanks @yanyongyu for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13. |
…t__ or __new__ (pythonGH-137862) Fixed several cases where __init__, __new__ or metaclass` __call__ is a descriptor that returns a wrapped function. (cherry picked from commit 025a213) Co-authored-by: Ju4tCode <[email protected]>
…t__ or __new__ (pythonGH-137862) Fixed several cases where __init__, __new__ or metaclass` __call__ is a descriptor that returns a wrapped function. (cherry picked from commit 025a213) Co-authored-by: Ju4tCode <[email protected]>
GH-138224 is a backport of this pull request to the 3.14 branch. |
GH-138225 is a backport of this pull request to the 3.13 branch. |
…t__ or __new__ (pythonGH-137862) Fixed several cases where __init__, __new__ or metaclass` __call__ is a descriptor that returns a wrapped function. (cherry picked from commit 025a213) Co-authored-by: Ju4tCode <[email protected]>
…it__ or __new__ (GH-137862) (GH-138225) Fixed several cases where __init__, __new__ or metaclass` __call__ is a descriptor that returns a wrapped function. (cherry picked from commit 025a213) Co-authored-by: Ju4tCode <[email protected]>
…t__ or __new__ (pythonGH-137862) Fixed several cases where __init__, __new__ or metaclass` __call__ is a descriptor that returns a wrapped function.
This PR fixes the
inspect.signature
behavior when inspecting a class that uses descriptor (like classmethod/staticmethod/custom) with a wrapped__init__
.This PR also adds more test cases not covered in PR #132055.
cyfunction __init__
due to unwrap and descriptor behavior changed #137317