Skip to content
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-81677: basic support for annotations in __text_signature__'s #101872

Open
wants to merge 23 commits into
base: main
Choose a base branch
from

Conversation

skirpichev
Copy link
Member

@skirpichev skirpichev commented Feb 13, 2023

  • add support for parsing annotations from __text_signature__ properties. Mostly naive and minimal implementation: there could be some validation of parsed annotations (e.g. to exclude something like print(1)).
    But I doubt, that this is required. In fact, probably the whole method could be refactored to do first parsing the program string, then do ast transformation (RewriteSymbolics) for defaults, then do ast.unparse(), eval its output and use _signature_from_function() helper to get the final result.
  • relax requirements for specification of text signatures in the PyDoc_STRVAR. Before - only ([<python-style parameter_list>]) was accepted. Now it permits everything till the end marker (\n--\n\n), that allows us to
    specify return annotations. (A first step to something like Support return annotation in signature for Argument Clinic #76120.)

This doesn't touch public interfaces: the __text_signature__ property left undocumented (but see #93865).

@skirpichev
Copy link
Member Author

@sobolevn, maybe you could review this (at least the inspect part)?

Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this!

Couple of thoughts:

  1. Does anyone actually use __text_annotations__? (I am asking because I don't know). I assume people use it, is it a breaking change for them?
  2. How well does your solution support more complex annotations that require (a) typing imports, what we might need from there? (b) forward references and https://peps.python.org/pep-0649/

Other than that - I don't really have an opinion on this feature. Because I don't any real use-cases for it.

@skirpichev
Copy link
Member Author

Does anyone actually use __text_annotations__?

I can't find any example outside of the CPython, and it's not documented (as the __signature__ property, which is used, e.g. in the SymPy). There is an issue, which is more or less a feature request: #93865

Clearly, it might be useful for extensions.

I assume people use it, is it a breaking change for them?

No, I don't think so. Previously, the return annotation was just ignored and if there were parameter annotations - an exception was raised.

How well does your solution support more complex annotations that require (a) typing imports, what we might need from there? (b) forward references

This solution does assume application in the stdlib modules and simple extensions (e.g. I've tested this on the gmpy2), so
nothing fancy yet. If this does make sense at all (see the subject on a different approach) - then I'll think about extending
it to a more complex annotations.

Because I don't any real use-cases for it.

Any example of using __text_signature__ property in the stdlib. In most cases, however, this will require
support from the AC (not sure if this should be a part of this PR). But some functions/methods add now signatures
without using the AC, e.g. the math.gcd.

@skirpichev
Copy link
Member Author

@1st1, you are listed as an expert for the inspect module. Maybe you could review this little pr?

@skirpichev
Copy link
Member Author

@vstinner, maybe you could review this? I would guess something like this should be a first step on solving #76120 (your issue). It seems some external projects already depend on this pr, so I would appreciate any feedback.

Lib/inspect.py Outdated Show resolved Hide resolved
Lib/inspect.py Outdated Show resolved Hide resolved
@skirpichev
Copy link
Member Author

https://discuss.python.org/t/43914

@skirpichev
Copy link
Member Author

PEP waiting for a sponsor: skirpichev/peps#2

@skirpichev skirpichev closed this Feb 20, 2024
@skirpichev skirpichev deleted the text_sign_annot branch February 20, 2024 09:39
@skirpichev skirpichev restored the text_sign_annot branch February 26, 2024 06:47
@skirpichev skirpichev reopened this Feb 26, 2024
@skirpichev skirpichev marked this pull request as draft February 26, 2024 06:48
@skirpichev skirpichev changed the title gh-81677: basic support for annotations in __text_signature__'s WIP: provide __signature__ attribute for builtins (Signature object built from the PEP 7-like "signature line") Feb 26, 2024
@skirpichev skirpichev changed the title WIP: provide __signature__ attribute for builtins (Signature object built from the PEP 7-like "signature line") gh-81677: WIP: provide __signature__ attribute for builtins (Signature object built from the PEP 7-like "signature line") Feb 26, 2024
@skirpichev skirpichev changed the title gh-81677: WIP: provide __signature__ attribute for builtins (Signature object built from the PEP 7-like "signature line") gh-81677: basic support for annotations in __text_signature__'s Jul 19, 2024
@skirpichev skirpichev marked this pull request as ready for review July 19, 2024 06:45
@skirpichev
Copy link
Member Author

Ok, it seems nobody like to sponsor PEP, so I'm returning PR to initial shape (that address review). This is ready for review again.

@AA-Turner
Copy link
Member

Ok, it seems nobody like to sponsor PEP

Have you posted the draft PEP on discuss.python.org? It may have been missed here...

A

@skirpichev
Copy link
Member Author

Have you posted the draft PEP on discuss.python.org? It may have been missed here...

Yes, that was posted in the thread, discussing this pr, there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting review interpreter-core (Objects, Python, Grammar, and Parser dirs) topic-typing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants