-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Autodoc: Support typing_extensions.overload
#13509
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: master
Are you sure you want to change the base?
Conversation
sphinx/pycode/parser.py
Outdated
if self.typing_final: | ||
final.append(self.typing_final) | ||
|
||
final = self.typing_final_names.copy() |
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.
Can you explain the .copy()
? Previously it always created an empty list.
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.
typing_final
was a str | None
before, I changed it to a list to store multiple seen names. This copies to avoid mutation. This is just a simplification compared to making an empty list, then extending it immediately.
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'd prefer to be explicit and use an empty list. The .copy()
makes local reasoning harder because the instance state might have been mutated elsewhere.
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.
Not sure what you mean by mutation elsewhere, unless other code made it not a list. Could also do list(self.typing_final_names)
. Using an empty list would be something like:
final = []
for name in self.typing_final_names:
final.append(name)
Which is pointlessly overcomplicated.
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.
Not sure what you mean by mutation elsewhere, unless other code made it not a list
Mainly the risk of having more entries than needed, becuase this is a list rather than a set. However I think making it a set would work?:
final = set(self.typing_final_names) | {f'{mod}.final' for mod in self.typing_mods}
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.
Ah, yes it can be possible for the list to have multiple copies, but only if the user did imports multiple times. Changing everything to sets would fix that, I just didn't think that was common enough to be worth using sets, when they'd normally only hold up to 4 items.
Purpose
I noticed that
autodoc
was not able to recognise@typing_extensions.overload
, so the extra signatures wouldn't appear. This PR makespycode.parser
able to detect this, also adding support for@typing_extensions.final
since it has the same issue.I'm not sure if
VariableCommentPicker
here is public or not, since I changed the attributes there to collect both possible names. If it is I could instead add another 3 attributes, but that's less elegant.