Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork1.9k
[networkx] More precise annotation for nested tuples#14287
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
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Uh oh!
There was an error while loading.Please reload this page.
| from networkx.classes.graph import Graph, _Node | ||
| from networkx.exception import NetworkXException | ||
| from networkx.utils.backends import _dispatchable | ||
| __all__ = ["from_nested_tuple", "from_prufer_sequence", "NotATree", "to_nested_tuple", "to_prufer_sequence"] | ||
| _NestedTuple: TypeAlias = tuple[_NestedTuple, ...] |
brianschubertJun 16, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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.
Should the RHS include a union with some other type? This currently only allows tuples with tuple entries (ultimately terminating with empty tuples)
Huh, apparently that's correct:
https://networkx.org/documentation/stable/reference/algorithms/generated/networkx.algorithms.tree.coding.from_nested_tuple.html#from-nested-tuple
Nevermind then! Sorry for the noise :-)
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.
Unless my understanding is wrong, there should indeed be no other types but only nested tuples:
https://networkx.org/documentation/stable/reference/algorithms/generated/networkx.algorithms.tree.coding.from_nested_tuple.html
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.
No worries, thanks for taking a look :)
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 was gonna ask the same question. And made the same assumption in my initial comment. Could you add a comment here pointing to that documentation ?
| _NestedTuple:TypeAlias=tuple[_NestedTuple, ...] | |
| # > The tree with one node and no edges is represented by the emptytuple, () | |
| # https://networkx.org/documentation/stable/reference/algorithms/generated/networkx.algorithms.tree.coding.from_nested_tuple.html#from-nested-tuple | |
| # So this really only recuses on tuples | |
| _NestedTuple:TypeAlias=tuple[_NestedTuple, ...] |
Feel free to reword.
(as a side note, you could write_NestedTuple: TypeAlias = tuple[_NestedTuple | tuple[()], ...], but that quickly becomes unusable/too unwieldy)
Otherwise LGTM.
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Brian Schubert <brianm.schubert@gmail.com>
According tomypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
| from networkx.classes.graph import Graph, _Node | ||
| from networkx.exception import NetworkXException | ||
| from networkx.utils.backends import _dispatchable | ||
| __all__ = ["from_nested_tuple", "from_prufer_sequence", "NotATree", "to_nested_tuple", "to_prufer_sequence"] | ||
| _NestedTuple: TypeAlias = tuple[_NestedTuple, ...] | ||
| class NotATree(NetworkXException): ... | ||
| @_dispatchable | ||
| def to_nested_tuple(T: Graph[_Node], root: _Node, canonical_form: bool = 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.
While we're here...
| defto_nested_tuple(T:Graph[_Node],root:_Node,canonical_form:bool=False): ... | |
| defto_nested_tuple(T:Graph[_Node],root:_Node,canonical_form:bool=False)->_NestedTuple: ... |
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.
Sounds good, I wasn't sure if the return hints were left out for a reason
| from networkx.classes.graph import Graph, _Node | ||
| from networkx.exception import NetworkXException | ||
| from networkx.utils.backends import _dispatchable | ||
| __all__ = ["from_nested_tuple", "from_prufer_sequence", "NotATree", "to_nested_tuple", "to_prufer_sequence"] | ||
| _NestedTuple: TypeAlias = tuple[_NestedTuple, ...] |
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 was gonna ask the same question. And made the same assumption in my initial comment. Could you add a comment here pointing to that documentation ?
| _NestedTuple:TypeAlias=tuple[_NestedTuple, ...] | |
| # > The tree with one node and no edges is represented by the emptytuple, () | |
| # https://networkx.org/documentation/stable/reference/algorithms/generated/networkx.algorithms.tree.coding.from_nested_tuple.html#from-nested-tuple | |
| # So this really only recuses on tuples | |
| _NestedTuple:TypeAlias=tuple[_NestedTuple, ...] |
Feel free to reword.
(as a side note, you could write_NestedTuple: TypeAlias = tuple[_NestedTuple | tuple[()], ...], but that quickly becomes unusable/too unwieldy)
Otherwise LGTM.
Followup to#14218 based on the suggestion by@Avasam.