-
-
Notifications
You must be signed in to change notification settings - Fork 33.3k
gh-91002: Support __annotate__ for functools.partial and functools.partialmethod
#139753
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?
Conversation
… inspect in annotationlib.get_annotations Signed-off-by: Manjusaka <[email protected]>
Signed-off-by: Manjusaka <[email protected]>
Signed-off-by: Manjusaka <[email protected]>
Signed-off-by: Manjusaka <[email protected]>
Signed-off-by: Manjusaka <[email protected]>
Signed-off-by: Manjusaka <[email protected]>
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.
Could this instead be implemented with a custom __annotate__ function on partial objects, so annotationlib itself doesn't need to change?
|
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 |
Hi @JelleZijlstra , thank you for your time. Currently, the partial C API doesn't have the relevant interfaces reserved. Adding them would be relatively more complex than the Python version. In short, if we solve this problem from functools, it would indeed be the optimal choice in terms of effect, but it would increase complexity. |
|
I'd strongly prefer if this is done in functools, not annotationlib. This sets a good example where objects can support annotation introspection themselves through overriding To make the implementation simpler, we can make the C code delegate to Python code, similar to how |
Nice, this is better than my original thought. I'll make a try |
Signed-off-by: Manjusaka <[email protected]>
Doc/library/annotationlib.rst
Outdated
| using ``getattr()`` and ``dict.get()`` for safety. | ||
| * For :class:`functools.partial` and :class:`functools.partialmethod` objects, | ||
| only returns annotations for parameters that have not been bound by the | ||
| partial application, along with the return annotation if present. |
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.
With the new implementation, I’m not sure how much annotationlib docs should mention support for partial. Maybe mention the generic support and link to a new section in functools doc with the info here?
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 have updated the docs
Doc/library/annotationlib.rst
Outdated
| although if *obj* is a wrapped function (using | ||
| :func:`functools.update_wrapper`) or a :class:`functools.partial` object, | ||
| :func:`functools.update_wrapper`), a :class:`functools.partial` object, | ||
| or a :class:`functools.partialmethod` object, |
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.
Are these three examples the only special cases handled, or is this paragraph saying that __wrapped__ attributes are followed to unwrap wrappers?
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 updated the docs, PTAL
Signed-off-by: Manjusaka <[email protected]>
__annotate__ for functools.partial and functools.partialmethod
Signed-off-by: Manjusaka <[email protected]>
|
I have made the requested changes; please review again |
|
Thanks for making the requested changes! @JelleZijlstra: please review the changes made to this pull request. |
Signed-off-by: Manjusaka <[email protected]>
Signed-off-by: Manjusaka <[email protected]>
|
|
||
| .. versionadded:: 3.14 | ||
|
|
||
| .. _functools-objects-annotations: |
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 feel like this doesn't belong in the annotationlib docs, maybe in the functools ones.
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 a different idea here. plz correct me if I'm wrong
It seems that we recommend people use annotationlib to process the annotation here. I prefer add a more detail document in here and reference this in functools
Signed-off-by: Manjusaka <[email protected]>
Signed-off-by: Manjusaka <[email protected]>
|
@JelleZijlstra I have updated the PR, PTAL |
📚 Documentation preview 📚: https://cpython-previews--139753.org.readthedocs.build/