Repository: networkx/networkx
Pull request: Update simple_paths.py: consistent behaviour for `is_simple_path` when path contains nodes not in the graph.
Current version of `is_simple_path` fails with `KeyError` if the first element of the node list is not in the graph.

Resolves https://github.com/networkx/networkx/issues/6271, by returning `False` if any node in the provided path is outside the graph.

Problem statement:
Current version of `is_simple_path` fails with `KeyError` if the first element of the node list is not in the graph.
Current version of `is_simple_path` fails with `KeyError` if the first element of the node list is not in the graph.

```python
from networkx import is_simple_path, path_graph

G = path_graph(4)

print(is_simple_path(G, [1, 10]))
# False

print(is_simple_path(G, [10, 1]))
# KeyError: 10
```

<!--- Provide a general summary of the issue in the Title above -->

### Current Behavior

Currently, the behavior seems to be inconsistent depending on whether the node outside the graph is the first node in the node list or appears later.

### Expected Behavior

If any node in the node list is not in the graph, the provide consistent response:
- Either raise `KeyError`
- or return `False`.

I am in favour of the latter.

Hints from issue discussion:

Commit message: Update simple_paths.py

Patch to review:
diff --git a/networkx/algorithms/simple_paths.py b/networkx/algorithms/simple_paths.py
index 6fb5cfee0c6..cd93e13da61 100644
--- a/networkx/algorithms/simple_paths.py
+++ b/networkx/algorithms/simple_paths.py
@@ -68,17 +68,24 @@ def is_simple_path(G, nodes):
     False
 
     """
+    assert isinstance(nodes, list), "Object passed as `nodes` must be a list."
+
     # The empty list is not a valid path. Could also return
     # NetworkXPointlessConcept here.
-    if len(nodes) == 0:
+    if not nodes:
+        return False
+
+    # check that all nodes in the list are in the graph, if at least one
+    # is not in the graph, then this is not a simple path
+    if not all(n in G for n in nodes):
         return False
-    # If the list is a single node, just check that the node is actually
-    # in the graph.
-    if len(nodes) == 1:
-        return nodes[0] in G
-    # Test that no node appears more than once, and that each
-    # adjacent pair of nodes is adjacent.
-    return len(set(nodes)) == len(nodes) and all(v in G[u] for u, v in pairwise(nodes))
+
+    # If the list contains repeated nodes, then it's not a simple path
+    if len(set(nodes)) != len(nodes):
+        return False
+
+    # Test that each adjacent pair of nodes is adjacent.
+    return all(v in G[u] for u, v in pairwise(nodes))
 
 
 def all_simple_paths(G, source, target, cutoff=None):
diff --git a/networkx/algorithms/tests/test_simple_paths.py b/networkx/algorithms/tests/test_simple_paths.py
index 08348b989c8..c2d4c008379 100644
--- a/networkx/algorithms/tests/test_simple_paths.py
+++ b/networkx/algorithms/tests/test_simple_paths.py
@@ -58,6 +58,10 @@ def test_missing_node(self):
         G = nx.path_graph(2)
         assert not nx.is_simple_path(G, [0, 2])
 
+    def test_missing_starting_node(self):
+        G = nx.path_graph(2)
+        assert not nx.is_simple_path(G, [2, 0])
+
     def test_directed_path(self):
         G = nx.DiGraph([(0, 1), (1, 2)])
         assert nx.is_simple_path(G, [0, 1, 2])


Code review comments:
diff_hunk: @@ -68,17 +68,24 @@ def is_simple_path(G, nodes):
     False
 
     """
+    assert isinstance(nodes, list), "Object passed as `nodes` must be a list."
+
     # The empty list is not a valid path. Could also return
     # NetworkXPointlessConcept here.
-    if len(nodes) == 0:
+    if not nodes:
+        return False
path: networkx/algorithms/simple_paths.py
text: @user1:
Similarly, I'm -1 on this too. It makes sense with the above, but not by itself.

diff_hunk: @@ -68,17 +68,24 @@ def is_simple_path(G, nodes):
     False
 
     """
+    assert isinstance(nodes, list), "Object passed as `nodes` must be a list."
path: networkx/algorithms/simple_paths.py
text: @user1:
-1 on this assertion - the docstring is clear on the expected type, and assertions are really intended to check that scenarios that should not ever exist are not hit. 

@author:
Thanks, makes sense!