diff --git a/releasenotes/notes/fix-neighbors-deterministic-order-3c8e1a9f4b2d7e05.yaml b/releasenotes/notes/fix-neighbors-deterministic-order-3c8e1a9f4b2d7e05.yaml new file mode 100644 index 0000000000..b3a221a58a --- /dev/null +++ b/releasenotes/notes/fix-neighbors-deterministic-order-3c8e1a9f4b2d7e05.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + Fixed :meth:`.PyGraph.neighbors`, :meth:`.PyDiGraph.neighbors`, and + :meth:`.PyDiGraph.neighbors_undirected` to return neighbors in a + deterministic order. Previously, calls to the functions could return + nodes in an arbitrary order. See + `#1501 `__. diff --git a/src/digraph.rs b/src/digraph.rs index f454dee2f9..fd601d7b90 100644 --- a/src/digraph.rs +++ b/src/digraph.rs @@ -1849,8 +1849,8 @@ impl PyDiGraph { .graph .neighbors(NodeIndex::new(node)) .map(|node| node.index()) - .collect::>() - .drain() + .collect::>() + .drain(..) .collect(), } } @@ -1866,7 +1866,7 @@ impl PyDiGraph { /// >>> G.neighbors_undirected(node) /// NodeIndices[3, 1] /// >>> G.to_undirected().neighbors(node) - /// NodeIndices[1, 3] + /// NodeIndices[3, 1] /// /// For direction-aware neighbors, see :func:`~predecessors` and :func:`~successors`. /// @@ -1881,8 +1881,8 @@ impl PyDiGraph { .graph .neighbors_undirected(NodeIndex::new(node)) .map(|node| node.index()) - .collect::>() - .drain() + .collect::>() + .drain(..) .collect(), } } diff --git a/src/graph.rs b/src/graph.rs index d9cd17611d..ed54430fc0 100644 --- a/src/graph.rs +++ b/src/graph.rs @@ -1186,8 +1186,8 @@ impl PyGraph { .graph .neighbors(NodeIndex::new(node)) .map(|node| node.index()) - .collect::>() - .drain() + .collect::>() + .drain(..) .collect(), } } diff --git a/tests/digraph/test_neighbors.py b/tests/digraph/test_neighbors.py index acd0ee590c..4be041d85d 100644 --- a/tests/digraph/test_neighbors.py +++ b/tests/digraph/test_neighbors.py @@ -79,3 +79,27 @@ def test_undirected_neighbors_cycle(self): undirected_neighbors = dag.neighbors_undirected(node) expected_neighbors = undirected_dag.neighbors(node) self.assertEqual(sorted(undirected_neighbors), sorted(expected_neighbors)) + + def test_neighbors_deterministic_order(self): + # Regression test for https://github.com/Qiskit/rustworkx/issues/1501: + # neighbors must be returned in a deterministic order. + dag = rustworkx.PyDiGraph() + dag.add_nodes_from(range(6)) + dag.add_edges_from_no_data([(0, 1), (0, 2), (0, 3)]) + dag.add_edges_from_no_data([(4, 0), (5, 0)]) + for _ in range(20): + self.assertEqual([3, 2, 1], list(dag.neighbors(0))) + self.assertEqual([3, 2, 1, 5, 4], list(dag.neighbors_undirected(0))) + + def test_neighbors_dedup_deterministic_order(self): + # Parallel edges are deduplicated with a stable order. + dag = rustworkx.PyDiGraph() + dag.add_nodes_from(["a", "b", "c"]) + dag.add_edge(0, 1, None) + dag.add_edge(0, 1, None) + dag.add_edge(0, 2, None) + first = list(dag.neighbors(0)) + self.assertCountEqual([1, 2], first) + self.assertEqual(2, len(first)) + for _ in range(20): + self.assertEqual(first, list(dag.neighbors(0))) diff --git a/tests/graph/test_neighbors.py b/tests/graph/test_neighbors.py index dca67c2556..e3a81872c9 100644 --- a/tests/graph/test_neighbors.py +++ b/tests/graph/test_neighbors.py @@ -41,3 +41,26 @@ def test_no_neighbor(self): graph = rustworkx.PyGraph() node_a = graph.add_node("a") self.assertEqual([], graph.neighbors(node_a)) + + def test_neighbors_deterministic_order(self): + # Regression test for https://github.com/Qiskit/rustworkx/issues/1501: + # neighbors must be returned in a deterministic order. + graph = rustworkx.PyGraph() + graph.add_nodes_from(range(6)) + graph.add_edges_from_no_data([(0, 1), (0, 2), (0, 3), (0, 4), (0, 5)]) + expected = [5, 4, 3, 2, 1] + for _ in range(20): + self.assertEqual(expected, list(graph.neighbors(0))) + + def test_neighbors_dedup_deterministic_order(self): + # Parallel edges are deduplicated, and the surviving order is stable. + graph = rustworkx.PyGraph() + graph.add_nodes_from(["a", "b", "c"]) + graph.add_edge(0, 1, None) + graph.add_edge(0, 1, None) + graph.add_edge(0, 2, None) + first = list(graph.neighbors(0)) + self.assertCountEqual([1, 2], first) + self.assertEqual(2, len(first)) + for _ in range(20): + self.assertEqual(first, list(graph.neighbors(0)))