bpo-46071: Improve graphlib documentation & reverse edge direction.

[ctrl-z-9000-times]

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

[the-knights-who-say-ni]

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 username

We couldn't find a bugs.python.org (b.p.o) account corresponding to the following GitHub usernames:

@ctrl-z-9000-times

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]

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.

[asvetlov]

Strictly speaking, the change is not backward compatible.
The current module allows keyword notation: `topo.add(node='a', 'b', 'c', 'd')

[ctrl-z-9000-times]

Ok, I reverted the name from start_node back to node.

I have made the requested changes; please review again

[bedevere-bot]

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 have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

[ctrl-z-9000-times]

I think you misunderstand:

1. I reserved the direction of the edges and also reversed the direction of the final sorted output, and those two reversals cancel each other out; so there are no logic changes needed.

2. I did not change the meaning of the word predecessors; I removed that word entirely.
   Instead I rewrote it to explicitly state the edge direction "from node X to node Y".

3. "The predecessor node is incoming, not outgoing."

According to the original docs it's actually the other way around:

https://docs.python.org/3/library/graphlib.html
If the optional graph argument is provided it must be a dictionary representing a directed acyclic graph where the keys are nodes and the values are iterables of all predecessors of that node in the graph (the nodes that have edges that point to the value in the key).

[ctrl-z-9000-times]

I have made the requested changes; please review again

[bedevere-bot]

Thanks for making the requested changes!

@asvetlov: please review the changes made to this pull request.

[ctrl-z-9000-times]

Actually, I think I'd like to add a few more sentences.

• I missed the word "successor" in method done(). I'm going to rephrase that using the terms "tasks" & "dependencies".
• The optional graph argument must explicitly state how to use it to store tasks & their dependencies, especially since I removed all of the "predecessor / successor" wording.
• The first paragraph could be a little more clear about the purpose of this class? It introduces task & dependency resolution as an example but, actually, a lot of what this class does is geared towards this use case. And some of the documentation does not make sense outside of the context of dependency resolution.
  • Maybe it could say something like "This class specializes in managing dependency resolution among tasks to be performed"

[asvetlov]

From my point of view, 'predecessor' is for 'incoming edge', not for 'outgoing'

[ctrl-z-9000-times]

I split this into two PR's:

#30223 - Reverses the edge direction

#30269 - General cleanup