[networkx] More precise annotation for nested tuples #14287
Conversation
|
|
||
| __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.
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.
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.
No worries, thanks for taking a look :)
There was a problem hiding this comment.
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 empty tuple, () | |
| # 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 <[email protected]>
|
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
| class NotATree(NetworkXException): ... | ||
|
|
||
| @_dispatchable | ||
| def to_nested_tuple(T: Graph[_Node], root: _Node, canonical_form: bool = False): ... |
There was a problem hiding this comment.
While we're here...
| def to_nested_tuple(T: Graph[_Node], root: _Node, canonical_form: bool = False): ... | |
| def to_nested_tuple(T: Graph[_Node], root: _Node, canonical_form: bool = False) -> _NestedTuple: ... |
There was a problem hiding this comment.
Sounds good, I wasn't sure if the return hints were left out for a reason
|
|
||
| __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.
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 empty tuple, () | |
| # 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.
3 participants
Followup to #14218 based on the suggestion by @Avasam.