provide Bool attribute to check for parents #145
Conversation
provide Bool attribute to check for parents
|
Hi, Could I propose a change?
|
| @@ -163,6 +163,7 @@ The following is an example that shows how to use this function: | |||
| The result has several fields, among which the following are most useful: | |||
|
|
|||
| * ``parents[i]``: the parent vertex of the i-th vertex. The parent of each source vertex is itself. | |||
| * ``hasparent[i]``: ``true`` if the i-th vertex has a parent, and ``false`` otherwise. When ``hasparent[i] == false``, it means that the vertex at index ``i`` isn't reachable from any source. Note that ``hasparent[i] == true`` for all source vertices. | |||
There was a problem hiding this comment.
Could we change this so that hasparent[i] is false for source vertices?
There was a problem hiding this comment.
Is there a reason for it? I thought this is consistent with the expectation that "the parent of each source is itself", and intended for hasparent to be a boolean mask for the parents attribute
There was a problem hiding this comment.
Yes. Technically, a node's parent is not itself unless there's an explicit edge between the node and itself, and that wouldn't ever exist (except possibly with A* and negative weights) in a shortest path calculation.
I don't know the original reason for making the source node its own parent, but it's not consistent with other packages (e.g., networkx) and will require an explicit check to ensure we're at the end of the (reversed) path so that we don't loop.
If we made this change, then the path enumeration can terminate on "hasparent=false".
There was a problem hiding this comment.
Yeah, I agree with you there. I'm just wondering whether this should be a local decision for hasparents (which I'll be happy to change), or whether it warrants opening an issue for the parents attribute as well
There was a problem hiding this comment.
let's continue the discussion over at #150, just to make sure that there aren't other users whose code depends on this behavior
replaces #144, and closes #140