[issue46071] Graphlib documentation (edge direction)
Change by David Mc Dougall : -- stage: patch review -> resolved status: open -> closed ___ Python tracker <https://bugs.python.org/issue46071> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue46466] help function reads comments
Change by David Mc Dougall : -- nosy: -dam1784 ___ Python tracker <https://bugs.python.org/issue46466> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue46466] help function reads comments
New submission from David Mc Dougall :
My inline comment ('#') got picked up by the help command.
Write the following code to a file (I named it "reproducer.py"):
"""
class Foo:
# Hello docstring, I'm a '#' comment!
def bar(self):
pass
assert Foo.bar.__doc__ is None
help(Foo.bar)
"""
The bug only happens when the file is executed.
$ python3 reproducer.py
Help on function bar in module __main__:
bar(self)
# Hello docstring, I'm a '#' comment!
Evaluating it at the interactive prompt does not reproduce the bug.
$ cat reproducer.py | python3
Help on function bar in module __main__:
bar(self)
--
messages: 411218
nosy: dam1784
priority: normal
severity: normal
status: open
title: help function reads comments
type: behavior
versions: Python 3.8
___
Python tracker
<https://bugs.python.org/issue46466>
___
___
Python-bugs-list mailing list
Unsubscribe:
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue46204] Graphlib documentation (general cleanup)
Change by David Mc Dougall : -- resolution: -> wont fix stage: patch review -> resolved status: open -> closed ___ Python tracker <https://bugs.python.org/issue46204> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue46071] Graphlib documentation (edge direction)
Change by David Mc Dougall : -- nosy: -dam1784 ___ Python tracker <https://bugs.python.org/issue46071> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue46071] Graphlib documentation (edge direction)
David Mc Dougall added the comment: > It seems David places more value on the idea of the concrete mapping > "pointing forwards" with respect to the abstract directed graph, while it > seems Tim places more value on the idea of the abstract mapping direction > corresponding to the final static order. These two goals are in conflict, > assuming we don't want to change the behavior. Yes, that's right. But the good news is that if you're willing to rewrite all of the documentation you probably can explain this in a clear and simple way, and without breaking compatibility. I say *you* because I'm not going to argue with you all about this anymore, especially with Tim being... --- Tim: you're conflating the words "predecessors" and "dependency". In some contexts they can be synonymous, but they are not the same thing. * Predecessor refers to one of the sides of a directed edge. * Dependency refers to a semantic relationship between two of the users' things. > The only possible topsort [...] For which see absolutely any text defining > the terms. >From wiki: "Precisely, a topological sort is a graph traversal in which each >node v is visited only after all its dependencies are visited." This definition doesn't say anything about the "predecessors" or how the graph is stored, or anything about "edge direction". I like this definition. > that's not a matter of preference, it's just plain wrong I know that there are many different ways to represent a graph, but your graph format *is just plain wrong.* Goodbye -- ___ Python tracker <https://bugs.python.org/issue46071> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue46071] Graphlib documentation (edge direction)
David Mc Dougall added the comment:
No, the code works fine. I just wish the docs weren't so muddled.
I wish the docs started by saying:
> The graph is a dict of {'start_node': ['end_nodes',]}
> The topological sorter puts the end_nodes before their start_nodes.
[note: this is what the code currently does]
Then the docs could introduce and use different terminology: "tasks" and their
"dependencies".
Everyone understands what those are.
And nowhere does it need to say the word "predecessors".
But honestly, I think I'm going to throw in the towel and give up on this
endeavor.
--
___
Python tracker
<https://bugs.python.org/issue46071>
___
___
Python-bugs-list mailing list
Unsubscribe:
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue46071] Graphlib documentation (edge direction)
David Mc Dougall added the comment: I can post literally hundreds of examples of directed graphs that are traversable in the forward direction. This might be the only one which is *only* traversable backwards. > As to the meaning of "point to" Here is one: If I have a pointer in memory, I might represent that with an arrow, Like: ADDRESS -> VALUE Or if I have a dictionary I might write: KEY -> VALUE But in the graph the edges are directed the opposite way: KEY <- VALUE The edges in the graph point in the opposite direction as the underlying memory pointers. This is unexpected and confusing. -- ___ Python tracker <https://bugs.python.org/issue46071> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue46071] Graphlib documentation (edge direction)
David Mc Dougall added the comment: > you're not actually confused. I was when I first read it! > the meanings of "predecessor" and "successor" are universally agreed upon I disagree. The universally agreed upon terms are "directed edge u -> v". It's not obvious if the "predecessor" should be the start or the end point of the edge, and this is why the docs explicitly state the edge direction: > 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 [...] the nodes that have edges that point to the value > in the key. -- ___ Python tracker <https://bugs.python.org/issue46071> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue46071] Graphlib documentation (edge direction)
David Mc Dougall added the comment:
> The argument passed is the predecessor form of the graph B -> A
where graph = {'A' : ['B']}
This is part that I'm objecting to. The form of the graph should be A -> B, not
B -> A.
The issue with the current form is that you can not traverse the graph, at
least not forwards. When I say traverse forwards I mean that you follow the
edges in the direction of the arrows. If you look up 'A' in the current graph
you get all of the nodes that point *to* A, but that doesn't help you get
*from* A to anywhere else.
There are two conventions:
1) Graphs should be traverse-able, by following the arrows.
2) Topological sorting makes the arrows point to the right.
Convention #1 was broken to satisfy convention #2.
What is important about the topo-sort is that it makes all of the edges point
in the *same* direction. It doesn't actually matter which direction that is.
And credit where due, the library picked the more-useful direction. It was a
pragmatic choice, driven by the real use case of dependency resolution.
But having the graph with arrows pointing in the wrong direction is going to
cause endless confusion.
For an example of the confusion this causes, look at the API itself. The "add"
method explicitly calls out the fact that you can add nodes with no
predecessors. This is obviously also possible with the graph argument as it
currently is.
> It is possible to add a node with no dependencies (predecessors is not
> provided)
https://docs.python.org/3/library/graphlib.html#graphlib.TopologicalSorter.add
--
___
Python tracker
<https://bugs.python.org/issue46071>
___
___
Python-bugs-list mailing list
Unsubscribe:
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue46071] Graphlib documentation (edge direction)
David Mc Dougall added the comment: > If the way the user collects their data stores only successor links (which, > as above, seems odd in applications that actually use topsorts), then they > need something like this instead: Actually they only need to do this: ts = TopologicalSorter(my_forward_graph).static_order() ts = reversed(ts) I think part of the issue here is that the document uses two terms to describe the same thing: "predecessor" and "edge-direction". Everywhere it discusses predecessors it gets it right, but the edge direction is hopelessly confused because they tried to use the "normal" definition of topo-sort and the only way to make that work is to also reverse the direction of the graph's edges to compensate (and the two reversals cancel each other out). The edge direction is only mentioned twice in the whole document, once to define topo-sort and again to define the graph format. If the users problem fits into the task dependency paradigm, then this library makes a lot of sense. But for users who just have a directed graph, the documentation is really really confusing. I think it would be much better if the document explained that this was a "reversed" topological sort with a slightly different definition, and used a "bog standard" graph format like for example in this tutorial: https://www.python.org/doc/essays/graphs/ -- ___ Python tracker <https://bugs.python.org/issue46071> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue46071] Graphlib documentation (edge direction)
David Mc Dougall added the comment: The "reverse-toposort" is actually quite a good idea. The end-user is usually going to want to iterate over the sorted output in the "reverse" order anyways, especially if they're doing task ordering / dependency resolution. Also, the underlying algorithm produces the "reverse" ordering by default. In my experience from writing and using my own topological sorting programs using the "correct" definition: the toposorter reverses the list, and then the users iterates over it in reverse order. -- ___ Python tracker <https://bugs.python.org/issue46071> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue46204] Graphlib documentation (general cleanup)
Change by David Mc Dougall : -- keywords: +patch pull_requests: +28515 stage: -> patch review pull_request: https://github.com/python/cpython/pull/30269 ___ Python tracker <https://bugs.python.org/issue46204> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue46204] Graphlib documentation (general cleanup)
New submission from David Mc Dougall : The graphlib documentation has some grammar & phrasing issues. -- components: Library (Lib) messages: 409370 nosy: dam1784 priority: normal severity: normal status: open title: Graphlib documentation (general cleanup) type: enhancement versions: Python 3.11 ___ Python tracker <https://bugs.python.org/issue46204> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue46071] Graphlib documentation (edge direction)
Change by David Mc Dougall : -- title: Graphlib documentation -> Graphlib documentation (edge direction) ___ Python tracker <https://bugs.python.org/issue46071> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue46071] Graphlib documentation
Change by David Mc Dougall : -- pull_requests: +28446 pull_request: https://github.com/python/cpython/pull/30223 ___ Python tracker <https://bugs.python.org/issue46071> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue46071] Graphlib documentation
New submission from David Mc Dougall : The documentation for graphlib encourages users to represent their graphs in a awkward format. Graphs are currently represented using dictionaries of nodes, for example: graph["end_node"] = ["start_node"] And this is unintuitive because you can't use the graph traverse edges in their forward direction. If you look up a node in the graph, you get all of the nodes that point to the key, as opposed to all of the nodes that the key points to. The solution is to rewrite the documentation such that all of the edge directions are reversed. The final topologically sorted list will be in the "opposite" direction as a consequence. This will cause no functional changes. -- assignee: docs@python components: Documentation messages: 408535 nosy: dam1784, docs@python priority: normal severity: normal status: open title: Graphlib documentation type: enhancement versions: Python 3.10, Python 3.11, Python 3.9 ___ Python tracker <https://bugs.python.org/issue46071> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
