-
Notifications
You must be signed in to change notification settings - Fork 164
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
Allow PyGraph and PyDiGraph to be annotated as generic classes at runtime #1348
Conversation
Pull Request Test Coverage Report for Build 12770141739Details
💛 - Coveralls |
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.
One quick question about the release note. But otherwise this lgtm it's a pretty straightforward feature and one I could see people using in typing contexts.
`PEP 560 <https://peps.python.org/pep-0560/>`__. Building off of the previous | ||
releases which introduced type annotations, the following code snippet is now valid: | ||
|
||
.. jupyter-execute:: |
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.
Does this need to be executed? It seems like a static code-block since we just need syntax highlighting
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 no preference either is fine. But I will say the advantage of executing is that it tests the code path.
If you execute that with 0.15.1 you’ll get a runtime error!
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'm fine leaving it. We're not super worried about our docs build time right now. If we were though this is the kind of thing we can change.
Fixes #1345. It should address the core aspects for
PyGraph
andPyDiGraph
. However, custom return types will come in a follow up PRThis makes the following work without requiring
from __future__ import annotations
:Also, this is worth highlighting: I refrained from addingI ended up adding the method to the stubs as__class_getitem__
to the type stubs as in the stubs that method is inherited fromGeneric[_S, _T]
mypy
stub tests disagreed with my reasoning