CycleError: Report all nodes #159
Conversation
pytools/graph.py
Outdated
| def __init__(self, node: T) -> None: | ||
| self.node = node | ||
| def __init__(self, nodes: List[T]) -> None: | ||
| self.nodes = nodes |
There was a problem hiding this comment.
This breaks backward compatibility.
There was a problem hiding this comment.
How can I avoid that - maybe by providing a nodes parameter (with a default of None) in addition to the node parameter?
There was a problem hiding this comment.
I think a better approach would be to make node a @property that spews a DeprecationWarning.
pytools/graph.py
Outdated
| raise CycleError(list(n for n, num_preds in | ||
| nodes_to_num_predecessors.items() if num_preds != 0)) |
There was a problem hiding this comment.
It would be nice to provide this in traversable (rather than arbitrary) order, which is doable at
There was a problem hiding this comment.
I'm not sure how to do that - how would we provide this in case there are multiple cycles in the graph?
|
Unsubscribing... @-mention or request review once it's ready for a look or needs attention. |
matthiasdiener
force-pushed
the
cycle-all
branch
from
282ed93 to
e877f4f
Compare
matthiasdiener
force-pushed
the
cycle-all
branch
from
e877f4f to
9d84d83
Compare
matthiasdiener
force-pushed
the
cycle-all
branch
from
9d84d83 to
bf70743
Compare
matthiasdiener
force-pushed
the
cycle-all
branch
from
de58acc to
c39afca
Compare
|
Also worth referring to the suggestions in this PR: https://gitlab.tiker.net/inducer/pytools/-/merge_requests/37. |
matthiasdiener
force-pushed
the
cycle-all
branch
from
5bbfbbb to
1462f72
Compare
pytools/graph.py
Outdated
| def __init__(self, node: T) -> None: | ||
| self.node = node | ||
| def __init__(self, nodes: List[T]) -> None: | ||
| self.nodes = nodes |
There was a problem hiding this comment.
I think a better approach would be to make node a @property that spews a DeprecationWarning.
pytools/graph.py
Outdated
| :attr node: Node in a directed graph that is part of a cycle. | ||
| :attr nodes: List of nodes in a directed graph that are part of a cycle. |
There was a problem hiding this comment.
Maybe path is a better name. Also state that we claim that node[i+1] in path is a successor of node[i] (and test that!).
3 participants
No description provided.