Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork33.3k
gh-101552: Allow pydoc to display signatures in source format#124669
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| ida=inspect_deferred_annotations | ||
| sig=inspect.Signature | ||
| par=inspect.Parameter | ||
| PORK=inspect.Parameter.POSITIONAL_OR_KEYWORD |
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.
🐷
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.
Nice! Some nitpicks, but nothing blocking really
Uh oh!
There was an error while loading.Please reload this page.
Doc/library/inspect.rst Outdated
| :func:`copy.replace`. | ||
| ..method::format(*, max_width=None) | ||
| ..method::format(*, max_width=None, unquote_annotations=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.
This doesn't feel like a totally correct use of "unquote" to me... Maybe the parameter could be calledremove_annotation_quotes instead ofunquote_annotation...? I think that might also reflect slightly better the fact that not all annotations will necessarily be quoted at all.
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.
We use "unquote" in various other places (e.g.,https://docs.python.org/3.13/library/csv.html#csv.QUOTE_NOTNULL); I feel it's more clear and concise
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.
"unquoted" is used inhttps://docs.python.org/3.13/library/csv.html#csv.QUOTE_NOTNULL as an adjective rather than a verb. I think it makes sense to talk about something being "unquoted" but less sense to talk about "unquoting" something.
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.
"dequote"?
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.
If I'm reading the code correctly, if this is false (the default) we userepr on annotation values that are strings, and if it's true we skiprepr only for strings. So this is a passive operation--if the value is true, wedon't do something, and when it's false wedo do something. I think that's why this is awkward.
I suggest the clearest way to express this is to flip it. Negate the boolean and rename itquote_annotations=True. If it's true (the default) we quote annotations, and if it's false we don't. I might even go with the wordierquote_annotation_strings=True, as we're only allowing the user to control quoting or not-quoting the annotations when they're strings. But I don't have a strong opinion about that.
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.
Good idea, changing that.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Uh oh!
There was an error while loading.Please reload this page.
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Doc/library/inspect.rst Outdated
| attempt to automatically un-stringize the annotations using | ||
| :func:`annotationlib.get_annotations`. The | ||
| *globals*, *locals*, and *eval_str* parameters are passed | ||
| *globals*, *locals*,*eval_str*,and *annotation_format* parameters are passed |
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.
It's a minor thing, but:globals,locals, andeval_str are passed in to parameters with the same name when callingget_annotations. Butannotation_format is passed in to a parameter with a different name (justformat). It might be nice to explicitly tell that to the user.
I realize the documentation doesn't explicitly detail where these parameters go, so this would be all-new text. And when I try it in my head it always comes across as clumsy. So... should we even bother to try? Would anybody be confused if we didn't bother to explain it?
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.
Good point, I will add a separate sentence about theannotation_format parameter.
Doc/library/inspect.rst Outdated
| :func:`copy.replace`. | ||
| ..method::format(*, max_width=None) | ||
| ..method::format(*, max_width=None, unquote_annotations=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.
If I'm reading the code correctly, if this is false (the default) we userepr on annotation values that are strings, and if it's true we skiprepr only for strings. So this is a passive operation--if the value is true, wedon't do something, and when it's false wedo do something. I think that's why this is awkward.
I suggest the clearest way to express this is to flip it. Negate the boolean and rename itquote_annotations=True. If it's true (the default) we quote annotations, and if it's false we don't. I might even go with the wordierquote_annotation_strings=True, as we're only allowing the user to control quoting or not-quoting the annotations when they're strings. But I don't have a strong opinion about that.
Misc/NEWS.d/next/Library/2024-09-27-06-39-32.gh-issue-101552.xYkzag.rst OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
7840638 intopython:mainUh oh!
There was an error while loading.Please reload this page.
…ython#124669)Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Python 3.14 changed formatannotation() function signature by adding `*,quote_annotation_strings=True` at the end of it.Since we only care about avoiding `text.removeprefix('typing.')`, let'sjust pass extra args/kwargs as is to keep compatibility with < 3.14.[1]python/cpython#124669Change-Id: Ic115ed5cc8766cdf60286e88f6cfa79e263f64c9Reviewed-by: Łukasz Patron <priv.luk@gmail.com>Reviewed-by: Christian Tismer <tismer@stackless.com>Reviewed-by: Friedemann Kleint <Friedemann.Kleint@qt.io>
Uh oh!
There was an error while loading.Please reload this page.
inspect.signature, matchingannotationlib.Formatinspect.Signature.format, allowing string annotations to be displayed without quoteshelp(...)should unstringify (and "unforwardref") annotations #101552📚 Documentation preview 📚:https://cpython-previews--124669.org.readthedocs.build/