GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — gh-101552: Allow pydoc to display signatures in source format by JelleZijlstra · Pull Request #124669 · python/cpython Conversation
|
| ida = inspect_deferred_annotations | ||
| sig = inspect.Signature | ||
| par = inspect.Parameter | ||
| PORK = inspect.Parameter.POSITIONAL_OR_KEYWORD |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
🐷
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Nice! Some nitpicks, but nothing blocking really
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
| :func:`copy.replace`. | ||
|
|
||
| .. method:: format(*, max_width=None) | ||
| .. method:: format(*, max_width=None, unquote_annotations=False) |
There was a problem 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 called remove_annotation_quotes instead of unquote_annotation...? I think that might also reflect slightly better the fact that not all annotations will necessarily be quoted at all.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem 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
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
"unquoted" is used in https://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.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
"dequote"?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem 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 use repr on annotation values that are strings, and if it's true we skip repr only for strings. So this is a passive operation--if the value is true, we don't do something, and when it's false we do 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 it quote_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 wordier quote_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.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Good idea, changing that.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Co-authored-by: Alex Waygood <[email protected]>
There was an error while loading. Please reload this page.
Co-authored-by: Alex Waygood <[email protected]>
| 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.
The reason will be displayed to describe this comment to others. Learn more.
It's a minor thing, but: globals, locals, and eval_str are passed in to parameters with the same name when calling get_annotations. But annotation_format is passed in to a parameter with a different name (just format). 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?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem 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 the annotation_format parameter.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| :func:`copy.replace`. | ||
|
|
||
| .. method:: format(*, max_width=None) | ||
| .. method:: format(*, max_width=None, unquote_annotations=False) |
There was a problem 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 use repr on annotation values that are strings, and if it's true we skip repr only for strings. So this is a passive operation--if the value is true, we don't do something, and when it's false we do 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 it quote_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 wordier quote_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.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
…ython#124669) Co-authored-by: Alex Waygood <[email protected]>
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's
just pass extra args/kwargs as is to keep compatibility with < 3.14.
[1] python/cpython#124669
Change-Id: Ic115ed5cc8766cdf60286e88f6cfa79e263f64c9
Reviewed-by: Łukasz Patron <[email protected]>
Reviewed-by: Christian Tismer <[email protected]>
Reviewed-by: Friedemann Kleint <[email protected]>
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.