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.
What do you think of c39afca?
There was a problem hiding this comment.
I think a better approach would be to make node a @property that spews a DeprecationWarning.
There was a problem hiding this comment.
Implemented in 8823bca
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?
There was a problem hiding this comment.
#167 has some work in this direction
|
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
| @@ -213,9 +213,17 @@ class CycleError(Exception): | |||
| Raised when a topological ordering cannot be computed due to a cycle. | |||
|
|
|||
| :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!).
No description provided.