fix trouble with init_cycle argument to two TSP functions (#4938)

2 files changed, 17 insertions, 10 deletions

diff --git a/networkx/algorithms/approximation/tests/test_traveling_salesman.py b/networkx/algorithms/approximation/tests/test_traveling_salesman.py
index 78d93351..f36981d4 100644
--- a/networkx/algorithms/approximation/tests/test_traveling_salesman.py
+++ b/networkx/algorithms/approximation/tests/test_traveling_salesman.py
@@ -210,6 +210,11 @@ class TestSimulatedAnnealingTSP:
         cost = sum(self.UG2[n][nbr]["weight"] for n, nbr in pairwise(cycle))
         validate_symmetric_solution(cycle, cost, self.UG2_cycle, self.UG2_cost)
 
+    def test_error_on_input_order_mistake(self):
+        # see issue #4846 https://github.com/networkx/networkx/issues/4846
+        pytest.raises(TypeError, self.tsp, self.UG, weight="weight")
+        pytest.raises(nx.NetworkXError, self.tsp, self.UG, "weight")
+
     def test_not_complete_graph(self):
         pytest.raises(
             nx.NetworkXError,
diff --git a/networkx/algorithms/approximation/traveling_salesman.py b/networkx/algorithms/approximation/traveling_salesman.py
index ad88f7f1..dc6964c8 100644
--- a/networkx/algorithms/approximation/traveling_salesman.py
+++ b/networkx/algorithms/approximation/traveling_salesman.py
@@ -447,13 +447,11 @@ def simulated_annealing_tsp(
         The distance between all pairs of nodes should be included.
 
     init_cycle : list of all nodes or "greedy"
-        The initial solution (a cycle through all nodes).
-        Usually you should use `greedy_tsp(G, weight)`.
-        But you can start with `list(G)` or the final result
-        of `simulated_annealing_tsp` when doing `threshold_accepting_tsp`.
-
-        This argument is required. A shortcut if you don't want to think
-        about it is to use the string "greedy" which calls `greedy_tsp`.
+        The initial solution (a cycle through all nodes returning to the start).
+        This argument has no default to make you think about it.
+        If "greedy", use `greedy_tsp(G, weight)`.
+        Other common starting cycles are `list(G) + [next(iter(G))]` or the final
+        result of `simulated_annealing_tsp` when doing `threshold_accepting_tsp`.
 
     weight : string, optional (default="weight")
         Edge data key corresponding to the edge weight.
@@ -663,6 +661,13 @@ def threshold_accepting_tsp(
         `G` should be a complete weighted undirected graph.
         The distance between all pairs of nodes should be included.
 
+    init_cycle : list or "greedy"
+        The initial solution (a cycle through all nodes returning to the start).
+        This argument has no default to make you think about it.
+        If "greedy", use `greedy_tsp(G, weight)`.
+        Other common starting cycles are `list(G) + [next(iter(G))]` or the final
+        result of `simulated_annealing_tsp` when doing `threshold_accepting_tsp`.
+
     weight : string, optional (default="weight")
         Edge data key corresponding to the edge weight.
         If any edge does not have this attribute the weight is set to 1.
@@ -713,9 +718,6 @@ def threshold_accepting_tsp(
         least one acceptance of a neighbor solution.
         If no inner loop moves are accepted the threshold remains unchanged.
 
-    cycle : list, optional (default=compute using greedy algorithm)
-        The initial solution (a cycle all nodes).
-
     seed : integer, random_state, or None (default)
         Indicator of random number generation state.
         See :ref:`Randomness<randomness>`.