bpo-46071: Improve graphlib documentation & reverse edge direction.#30102
bpo-46071: Improve graphlib documentation & reverse edge direction.#30102ctrl-z-9000-times wants to merge 2 commits intopython:mainfrom ctrl-z-9000-times:main
Conversation
This reverses the direction of the edges, and alters the definition of topological sorting accordingly. This should make the interface much more intuitive to use. I also rephrased most of the text for clarity. No functional changes.
|
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA). Recognized GitHub usernameWe couldn't find a bugs.python.org (b.p.o) account corresponding to the following GitHub usernames: This might be simply due to a missing "GitHub Name" entry in one's b.p.o account settings. This is necessary for legal reasons before we can look at this contribution. Please follow the steps outlined in the CPython devguide to rectify this issue. You can check yourself to see if the CLA has been received. Thanks again for the contribution, we look forward to reviewing it! |
asvetlov
left a comment
There was a problem hiding this comment.
Thanks for the PR.
I don't think that wild changes in already used terms make sense.
The predecessor node is incoming, not outgoing.
It cannot be changed just in docs without internal logic updating (conditions when a node becomes 'ready').
The logic update, in turn, is not backward compatible.
Lib/graphlib.py
Outdated
|
|
||
| def add(self, node, *predecessors): | ||
| """Add a new node and its predecessors to the graph. | ||
| def add(self, start_node, *end_nodes): |
There was a problem hiding this comment.
Strictly speaking, the change is not backward compatible.
The current module allows keyword notation: `topo.add(node='a', 'b', 'c', 'd')
There was a problem hiding this comment.
Ok, I reverted the name from start_node back to node.
I have made the requested changes; please review again
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
|
I think you misunderstand:
According to the original docs it's actually the other way around:
|
|
I have made the requested changes; please review again |
|
Thanks for making the requested changes! @asvetlov: please review the changes made to this pull request. |
|
Actually, I think I'd like to add a few more sentences.
|
asvetlov
left a comment
There was a problem hiding this comment.
From my point of view, 'predecessor' is for 'incoming edge', not for 'outgoing'
This reverses the direction of the edges, and alters the definition of
topological sorting accordingly. This should make the interface much
more intuitive to use.
I also rephrased most of the text for clarity.
No functional changes.
https://bugs.python.org/issue46071