Index edges for multi graph simple paths

[MartinPJorge]

Modifies _all_simple_paths_multigraph() to return the traversed edges rather than the nodes.
The returned edges contain their associated keys, so user can distinguish across edges connecting same pair of nodes.

If we have following MultiGraph with these nodes and edges (with their keys):

   (1,2,'k0')
    ________
  /          \                     
1 ___________ 2  ___________ 3
   (1,2,'k1')    (2,3,'k0')

The modified _all_simple_paths_multigraph() now returns:

import networkx as nx

mg = nx.MultiGraph()
mg.add_edge(1,2,key='k0')
mg.add_edge(1,2,key='k1')
mg.add_edge(2,3,key='k0')
paths = nx.all_simple_paths(mg, source=1, target=3)
print(list(paths))

>>> [[(1,2,'k0'),(2,3,'k0')], [(1,2,'k1'),(2,3,'k0')]]

rather than

>>> [[1,2,3],[1,2,3]]

[dschult]

Hmmm... This PR points out that we use node-paths. That is, we store and report paths as lists of nodes rather than lists of edges. There is a bijection for simple graphs between node-paths and edge-paths. But for MultiGraphs, node-paths do not uniquely specify edge-paths.

I think mixing the two output structures will lead to a convoluted API. Sometimes a function will return a list of nodes and other times a list of edges. That's going to be hard for users to debug and for us to maintain.

Does anyone have any comments on paths specified as lists for edges?

How hard would it be to make a new function that returns simple paths as edge-paths, that is as lists of edges instead of lists of nodes. For non-multigraphs it could use the current functions and then convert to edge-paths. For multigraphs it would return what you've put into this PR.
What do you think?

[MartinPJorge]

How hard would it be to make a new function that returns simple paths as edge-paths, that is as lists of edges instead of lists of nodes. For non-multigraphs it could use the current functions and then convert to edge-paths. For multigraphs it would return what you've put into this PR.

That might be the best solution. I think it should not take much effort, since for non multi graphs one can use nx.utils.pairwise to get a list of edges from a given list of nodes, e.g.,

G = nx.complete_graph(4)
paths = nx.all_simple_paths(G, source=0, target=3)
for path in map(nx.utils.pairwise, paths):
  print(list(path))

[(0, 1), (1, 2), (2, 3)]
[(0, 1), (1, 3)]
[(0, 2), (2, 1), (1, 3)]
[(0, 2), (2, 3)]
[(0, 3)]

I can create a function called all_simple_paths_edges() that mimics this behaviour for non-multi graphs, and uses the function of the pull request for multi graphs. Something like this:

# Non multi graph
G = nx.complete_graph(4)
paths_edges = nx.all_simple_paths_edges(G, source=0, target=3)
for path_edges in paths_edges:
  print(list(path_edges))

[(0, 1), (1, 2), (2, 3)]
[(0, 1), (1, 3)]
[(0, 2), (2, 1), (1, 3)]
[(0, 2), (2, 3)]
[(0, 3)]

# multi graph
mg = nx.MultiGraph()
mg.add_edge(1,2,key='k0')
mg.add_edge(1,2,key='k1')
mg.add_edge(2,3,key='k0')
paths_edges = nx.all_simple_paths_edges(mg, source=1, target=3)
print(list(paths_edges))

[[(1,2,'k0'),(2,3,'k0')], [(1,2,'k1'),(2,3,'k0')]]

Do you agree?

[dschult]

Yes -- that looks good to me. Let's think about name (you get to choose):
paths_edges, path_edges, edge_paths [with the all_simple in front, but I think the version without all_simple will become a name for the data-structure of specifying a path by making a list of edges.
What should we call a list of edges that specify a path.

:)

[MartinPJorge]

If latter on there will be a data-structure for a path as a list of edges, maybe it is better choosing all_simple_edge_paths(). It sounds better an edge_path data-structure than a path_edge data-structure (at least to me).

I'll start writing the all_simple_edge_paths() as soon as possible. Latter on we can include the path_edge data-structure.

---

Problem: My concern is that the naming can lead to confusions, since one can think that all_simple_edge_paths() will return a path without repeated edges, which is known as a trail in graph theory. I created a pull request to compute trails in directed graphs: #3393

[dschult]

I saw the PR on trails. Looks like it has some testing errors on travis (but apparently not on appveyor).

Let's see if we can make it clear: And just so I check what I think I know:
A trail is a path that doesn't repeat edges. A simple path is a path that doesn't revisit any nodes.
Right?

all_simple_edge_paths(G) Use first full paragraph of the doc_string to define "simple path" and to say we return edge_paths. Then, even if the name is not 100% clear, the docs are.

all_simple_paths(G) and all_trails(G) return paths as lists of nodes.

I suppose we could create something like: all_trails_as_edge_paths if it comes to that.

[MartinPJorge]

Perfect! I definitely agree on that. :D

[MartinPJorge]

Hi,

Last commit includes all_simple_edge_paths as discussed.
In the other pull request I coded the all_trails and all_trails_as_edge_paths.

Both pull requests, this and the mentioned above, just have Travis error in Python 3.5 in the docstring. The problem is related to the order in which the docstring code examples present the output, vs. the expected order. Honestly I don't think it's an error, because even python 3.5 obtains a correct output.

Do you think it's ready to accept the pull requests #3393 #3358?

[dschult]

Same comment as on the other PR.
Perhaps a sorted() function will help the test results.
We need to make the tests pass independent of the order of
dict reporting (and thus order of Graph reporting).

[MartinPJorge]

Done!

Including sorted() did the job.
This PR and #3393 pass the checks and have no conflicts.

Now it's ready to be safely merged.

[dschult]

Great -- Thanks for this. Some things to change:

• First line in doc_string should be on one line (<80 chars). Maybe:

  """Generate lists of edges for all simple paths in G from source to target

• In the first example in the doc_string, use integers instead of strings that look like integers. '1' -> 1

• same example, use g.add_egdes_from([(1, 2), (2, 4), (1, 3), (3, 4)])

• you don't need import networkx as nx in the examples. It is automagically included.

• remove blank lines in examples created by >>>

• remove spurious lines added to test_simple_paths.py

• add some tests?

[jtrakk]

This PR may be helpful in constructing a solution to shortest paths in MultiDiGraphs. #1120

[pep8speaks]

Hello @MartinPJorge! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

• In the file networkx/algorithms/tests/test_simple_paths.py:

Line 255:9: E123 closing bracket does not match indentation of opening bracket's line
Line 264:9: E123 closing bracket does not match indentation of opening bracket's line
Line 273:9: E123 closing bracket does not match indentation of opening bracket's line
Line 282:9: E123 closing bracket does not match indentation of opening bracket's line
Line 328:9: E123 closing bracket does not match indentation of opening bracket's line
Line 333:9: E123 closing bracket does not match indentation of opening bracket's line
Line 337:9: E123 closing bracket does not match indentation of opening bracket's line
Line 349:9: E123 closing bracket does not match indentation of opening bracket's line

Comment last updated at 2020-07-07 16:02:37 UTC