From 6092ebf2454f2992aca314fea5e8c0f2b751b942 Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Fri, 10 Feb 2023 15:49:03 -0500 Subject: [PATCH 01/10] Add PyDiGraph method to make edges symmetric This commit adds a new method make_symmetric() to PyDiGraph which will modify the graph and add a reverse edge to each edge in the graph if it is not already present. --- src/digraph.rs | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/src/digraph.rs b/src/digraph.rs index 0fcddde12..168e22e68 100644 --- a/src/digraph.rs +++ b/src/digraph.rs @@ -2606,6 +2606,46 @@ impl PyDiGraph { edges.is_empty() } + /// Make edges in graph symmetric + /// + /// This function iterates over all the edges in the graph and for each edge if a reverse + /// edge is not present in the graph one will be added. + /// + /// :param callable edge_payload: This optional argument takes in a callable which will + /// be passed a single positional argument the data payload for an edge that will + /// have a reverse copied in the graph. The returned value from this callable will + /// be used as the data payload for the new edge created. If this is not specified + /// then by default the data payload will be copied when the reverse edge is added. + /// If there are parallel edges, then one of the edges (typically the one with the lower + /// index, but this is not a guarantee) will be copied. + pub fn make_symmetric( + &mut self, + py: Python, + edge_payload_fn: Option, + ) -> PyResult<()> { + let edge_iter_vec: Vec<([NodeIndex; 2], EdgeIndex)> = self + .graph + .edge_references() + .map(|edge| ([edge.source(), edge.target()], edge.id())) + .collect(); + let mut edges: HashMap<[NodeIndex; 2], EdgeIndex> = edge_iter_vec.iter().copied().collect(); + for (edge_endpoints, edge_index) in edge_iter_vec { + let reverse_edge = [edge_endpoints[1], edge_endpoints[0]]; + if !edges.contains_key(&reverse_edge) { + let forward_weight = self.graph.edge_weight(edge_index).unwrap(); + let weight: PyObject = match edge_payload_fn.as_ref() { + Some(callback) => callback.call1(py, (forward_weight,))?, + None => forward_weight.clone_ref(py), + }; + let new_index = self + .graph + .add_edge(reverse_edge[0], reverse_edge[1], weight); + edges.insert(reverse_edge, new_index); + } + } + Ok(()) + } + /// Generate a new PyGraph object from this graph /// /// This will create a new :class:`~rustworkx.PyGraph` object from this From 56b075a3f49d3941190739e38f4c1ad44398537c Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Fri, 10 Feb 2023 20:21:16 -0500 Subject: [PATCH 02/10] Simplyify logic --- src/digraph.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/digraph.rs b/src/digraph.rs index 168e22e68..508c80aa9 100644 --- a/src/digraph.rs +++ b/src/digraph.rs @@ -2623,24 +2623,21 @@ impl PyDiGraph { py: Python, edge_payload_fn: Option, ) -> PyResult<()> { - let edge_iter_vec: Vec<([NodeIndex; 2], EdgeIndex)> = self + let edges: HashMap<[NodeIndex; 2], EdgeIndex> = self .graph .edge_references() .map(|edge| ([edge.source(), edge.target()], edge.id())) .collect(); - let mut edges: HashMap<[NodeIndex; 2], EdgeIndex> = edge_iter_vec.iter().copied().collect(); - for (edge_endpoints, edge_index) in edge_iter_vec { + for (edge_endpoints, edge_index) in edges.iter() { let reverse_edge = [edge_endpoints[1], edge_endpoints[0]]; if !edges.contains_key(&reverse_edge) { - let forward_weight = self.graph.edge_weight(edge_index).unwrap(); + let forward_weight = self.graph.edge_weight(*edge_index).unwrap(); let weight: PyObject = match edge_payload_fn.as_ref() { Some(callback) => callback.call1(py, (forward_weight,))?, None => forward_weight.clone_ref(py), }; - let new_index = self - .graph + self.graph .add_edge(reverse_edge[0], reverse_edge[1], weight); - edges.insert(reverse_edge, new_index); } } Ok(()) From 35a75873de28bb067bda09110eb115cb9890b579 Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Fri, 10 Feb 2023 20:34:18 -0500 Subject: [PATCH 03/10] Add initial tests --- .../rustworkx_tests/digraph/test_symmetric.py | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tests/rustworkx_tests/digraph/test_symmetric.py b/tests/rustworkx_tests/digraph/test_symmetric.py index 67f336d9f..d422183d3 100644 --- a/tests/rustworkx_tests/digraph/test_symmetric.py +++ b/tests/rustworkx_tests/digraph/test_symmetric.py @@ -37,3 +37,37 @@ def test_bidirectional_ring(self): ] digraph.extend_from_edge_list(edge_list) self.assertTrue(digraph.is_symmetric()) + + def test_empty_graph_make_symmetric(self): + digraph = rustworkx.PyDiGraph() + digraph.make_symmetric() + self.assertEqual(0, digraph.num_edges()) + self.assertEqual(0, digraph.num_nodes()) + + def test_path_graph_make_symmetric(self): + digraph = rustworkx.generators.directed_path_graph(4) + digraph.make_symmetric() + expected_edge_list = [ + (0, 1), + (1, 2), + (2, 3), + (1, 0), + (2, 1), + (3, 2), + ] + self.assertEqual(digraph.edge_list(), expected_edge_list) + + def test_path_graph_make_symmetric_existing_reverse_edges(self): + digraph = rustworkx.generators.directed_path_graph(4) + digraph.add_edge(3, 2, None) + digraph.add_edge(1, 0, None) + digraph.make_symmetric() + expected_edge_list = [ + (0, 1), + (1, 2), + (2, 3), + (3, 2), + (1, 0), + (2, 1), + ] + self.assertEqual(digraph.edge_list(), expected_edge_list) From acae85f05623956f2cc11b928337205a8d641aba Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Wed, 10 May 2023 09:02:52 -0400 Subject: [PATCH 04/10] Update docstring wording Co-authored-by: John Lapeyre --- src/digraph.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/digraph.rs b/src/digraph.rs index 508c80aa9..1c2f62082 100644 --- a/src/digraph.rs +++ b/src/digraph.rs @@ -2608,8 +2608,8 @@ impl PyDiGraph { /// Make edges in graph symmetric /// - /// This function iterates over all the edges in the graph and for each edge if a reverse - /// edge is not present in the graph one will be added. + /// This function iterates over all the edges in the graph, adding for each + /// edge the reversed edge, unless one is already present. /// /// :param callable edge_payload: This optional argument takes in a callable which will /// be passed a single positional argument the data payload for an edge that will From 2a490bf8fe6c4f34145a7f13c638ecd4f038419d Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Wed, 10 May 2023 09:09:55 -0400 Subject: [PATCH 05/10] Add release notes --- ...-digraph-make-symmetric-60d0287a7f7eec04.yaml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 releasenotes/notes/add-digraph-make-symmetric-60d0287a7f7eec04.yaml diff --git a/releasenotes/notes/add-digraph-make-symmetric-60d0287a7f7eec04.yaml b/releasenotes/notes/add-digraph-make-symmetric-60d0287a7f7eec04.yaml new file mode 100644 index 000000000..ce24c0f66 --- /dev/null +++ b/releasenotes/notes/add-digraph-make-symmetric-60d0287a7f7eec04.yaml @@ -0,0 +1,16 @@ +--- +features: + - | + Added a new method, :meth:`~.PyDiGraph.make_symmetric`, to the + :class:`~.PyDiGraph` class. This method is used to make all the edges + in the graph symmetric (there is a reverse edge in the graph for each edge). + For example: + + .. jupyter-execute:: + + import rustworkx as rx + from rustworkx.visualization import graphviz_draw + + graph = rx.generators.directed_path_graph(5, bidirectional=False) + graph.make_symmetric() + graphviz_draw(graph) From 4a8bab2a440908e2d0807fb37591714e402ea097 Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Wed, 10 May 2023 09:14:31 -0400 Subject: [PATCH 06/10] Expand testing --- .../rustworkx_tests/digraph/test_symmetric.py | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/tests/rustworkx_tests/digraph/test_symmetric.py b/tests/rustworkx_tests/digraph/test_symmetric.py index d422183d3..b25c6b231 100644 --- a/tests/rustworkx_tests/digraph/test_symmetric.py +++ b/tests/rustworkx_tests/digraph/test_symmetric.py @@ -71,3 +71,47 @@ def test_path_graph_make_symmetric_existing_reverse_edges(self): (2, 1), ] self.assertEqual(digraph.edge_list(), expected_edge_list) + + def test_empty_graph_make_symmetric_with_function_arg(self): + digraph = rustworkx.PyDiGraph() + digraph.make_symmetric(lambda _: "Reversi") + self.assertEqual(0, digraph.num_edges()) + self.assertEqual(0, digraph.num_nodes()) + + def test_path_graph_make_symmetric_with_function_arg(self): + digraph = rustworkx.generators.directed_path_graph(4) + digraph.make_symmetric(lambda: _: "Reversi") + expected_edge_list = [ + (0, 1, "Reversi"), + (1, 2, "Reversi"), + (2, 3, "Reversi"), + (1, 0, "Reversi"), + (2, 1, "Reversi"), + (3, 2, "Reversi"), + ] + self.assertEqual(digraph.weighted_edge_list(), expected_edge_list) + + def test_path_graph_make_symmetric_existing_reverse_edges_function_arg(self): + digraph = rustworkx.generators.directed_path_graph(4) + digraph.add_edge(3, 2, None) + digraph.add_edge(1, 0, None) + digraph.make_symmetric(lambda _: "Reversi") + expected_edge_list = [ + (0, 1, "Reversi"), + (1, 2, "Reversi"), + (2, 3, "Reversi"), + (3, 2, None), + (1, 0, None), + (2, 1, "Reversi"), + ] + self.assertEqual(digraph.weighted_edge_list(), expected_edge_list) + + def test_path_graph_make_symmetric_function_arg_raises(self): + digraph = rustworkx.generators.directed_path_graph(4) + + def weight_function(edge): + if edge is None: + raise TypeError("I'm expected") + + with self.assertRaises(TypeError): + digraph.make_symmetric(weight_function) From 722812c15b3829c29ccac9fdd940b7cc1e30858a Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Wed, 10 May 2023 10:02:12 -0400 Subject: [PATCH 07/10] Fix tests and cycle checking --- src/digraph.rs | 7 ++- .../rustworkx_tests/digraph/test_symmetric.py | 57 +++++++++++-------- 2 files changed, 36 insertions(+), 28 deletions(-) diff --git a/src/digraph.rs b/src/digraph.rs index 6ff04f1b1..1a6266b31 100644 --- a/src/digraph.rs +++ b/src/digraph.rs @@ -2714,7 +2714,9 @@ impl PyDiGraph { /// Make edges in graph symmetric /// /// This function iterates over all the edges in the graph, adding for each - /// edge the reversed edge, unless one is already present. + /// edge the reversed edge, unless one is already present. Note the edge insertion + /// is not fixed and the edge indices are not guaranteed to be consistent + /// between executions of this method on identical graphs. /// /// :param callable edge_payload: This optional argument takes in a callable which will /// be passed a single positional argument the data payload for an edge that will @@ -2741,8 +2743,7 @@ impl PyDiGraph { Some(callback) => callback.call1(py, (forward_weight,))?, None => forward_weight.clone_ref(py), }; - self.graph - .add_edge(reverse_edge[0], reverse_edge[1], weight); + self._add_edge(reverse_edge[0], reverse_edge[1], weight)?; } } Ok(()) diff --git a/tests/rustworkx_tests/digraph/test_symmetric.py b/tests/rustworkx_tests/digraph/test_symmetric.py index b25c6b231..b8e802029 100644 --- a/tests/rustworkx_tests/digraph/test_symmetric.py +++ b/tests/rustworkx_tests/digraph/test_symmetric.py @@ -15,6 +15,11 @@ import rustworkx +def default_weight_function(edge): + print(edge) + return "Reversi" + + class TestSymmetric(unittest.TestCase): def test_single_neighbor(self): digraph = rustworkx.PyDiGraph() @@ -45,66 +50,68 @@ def test_empty_graph_make_symmetric(self): self.assertEqual(0, digraph.num_nodes()) def test_path_graph_make_symmetric(self): - digraph = rustworkx.generators.directed_path_graph(4) + digraph = rustworkx.generators.directed_path_graph(4, bidirectional=False) digraph.make_symmetric() - expected_edge_list = [ + expected_edge_list = { (0, 1), (1, 2), (2, 3), (1, 0), (2, 1), (3, 2), - ] - self.assertEqual(digraph.edge_list(), expected_edge_list) + } + self.assertEqual(set(digraph.edge_list()), expected_edge_list) def test_path_graph_make_symmetric_existing_reverse_edges(self): - digraph = rustworkx.generators.directed_path_graph(4) + digraph = rustworkx.generators.directed_path_graph(4, bidirectional=False) digraph.add_edge(3, 2, None) digraph.add_edge(1, 0, None) digraph.make_symmetric() - expected_edge_list = [ + expected_edge_list = { (0, 1), (1, 2), (2, 3), (3, 2), (1, 0), (2, 1), - ] - self.assertEqual(digraph.edge_list(), expected_edge_list) + } + self.assertEqual(set(digraph.edge_list()), expected_edge_list) def test_empty_graph_make_symmetric_with_function_arg(self): digraph = rustworkx.PyDiGraph() - digraph.make_symmetric(lambda _: "Reversi") + digraph.make_symmetric(default_weight_function) self.assertEqual(0, digraph.num_edges()) self.assertEqual(0, digraph.num_nodes()) def test_path_graph_make_symmetric_with_function_arg(self): - digraph = rustworkx.generators.directed_path_graph(4) - digraph.make_symmetric(lambda: _: "Reversi") - expected_edge_list = [ - (0, 1, "Reversi"), - (1, 2, "Reversi"), - (2, 3, "Reversi"), + digraph = rustworkx.generators.directed_path_graph(4, bidirectional=False) + print(digraph.weighted_edge_list()) + digraph.make_symmetric(default_weight_function) + expected_edge_list = { + (0, 1, None), + (1, 2, None), + (2, 3, None), (1, 0, "Reversi"), (2, 1, "Reversi"), (3, 2, "Reversi"), - ] - self.assertEqual(digraph.weighted_edge_list(), expected_edge_list) + } + result = set(digraph.weighted_edge_list()) + self.assertEqual(result, expected_edge_list) def test_path_graph_make_symmetric_existing_reverse_edges_function_arg(self): - digraph = rustworkx.generators.directed_path_graph(4) + digraph = rustworkx.generators.directed_path_graph(4, bidirectional=False) digraph.add_edge(3, 2, None) digraph.add_edge(1, 0, None) - digraph.make_symmetric(lambda _: "Reversi") - expected_edge_list = [ - (0, 1, "Reversi"), - (1, 2, "Reversi"), - (2, 3, "Reversi"), + digraph.make_symmetric(default_weight_function) + expected_edge_list = { + (0, 1, None), + (1, 2, None), + (2, 3, None), (3, 2, None), (1, 0, None), (2, 1, "Reversi"), - ] - self.assertEqual(digraph.weighted_edge_list(), expected_edge_list) + } + self.assertEqual(set(digraph.weighted_edge_list()), expected_edge_list) def test_path_graph_make_symmetric_function_arg_raises(self): digraph = rustworkx.generators.directed_path_graph(4) From 4fa27ece406729803028f7e9d70c242e8d8989c2 Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Wed, 10 May 2023 10:03:38 -0400 Subject: [PATCH 08/10] Remove stray debug prints --- tests/rustworkx_tests/digraph/test_symmetric.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/rustworkx_tests/digraph/test_symmetric.py b/tests/rustworkx_tests/digraph/test_symmetric.py index b8e802029..3c29f0d55 100644 --- a/tests/rustworkx_tests/digraph/test_symmetric.py +++ b/tests/rustworkx_tests/digraph/test_symmetric.py @@ -16,7 +16,6 @@ def default_weight_function(edge): - print(edge) return "Reversi" @@ -85,7 +84,6 @@ def test_empty_graph_make_symmetric_with_function_arg(self): def test_path_graph_make_symmetric_with_function_arg(self): digraph = rustworkx.generators.directed_path_graph(4, bidirectional=False) - print(digraph.weighted_edge_list()) digraph.make_symmetric(default_weight_function) expected_edge_list = { (0, 1, None), From a7ee021c09b3022fd45896bd5fb588c7600b56e9 Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Fri, 12 May 2023 09:20:43 -0400 Subject: [PATCH 09/10] Update src/digraph.rs Co-authored-by: John Lapeyre --- src/digraph.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/digraph.rs b/src/digraph.rs index 8c8e861d8..0af1a5c27 100644 --- a/src/digraph.rs +++ b/src/digraph.rs @@ -2735,15 +2735,14 @@ impl PyDiGraph { .edge_references() .map(|edge| ([edge.source(), edge.target()], edge.id())) .collect(); - for (edge_endpoints, edge_index) in edges.iter() { - let reverse_edge = [edge_endpoints[1], edge_endpoints[0]]; - if !edges.contains_key(&reverse_edge) { + for ([edge_source, edge_target], edge_index) in edges.iter() { + if !edges.contains_key(&[*edge_target, *edge_source]) { let forward_weight = self.graph.edge_weight(*edge_index).unwrap(); let weight: PyObject = match edge_payload_fn.as_ref() { Some(callback) => callback.call1(py, (forward_weight,))?, None => forward_weight.clone_ref(py), }; - self._add_edge(reverse_edge[0], reverse_edge[1], weight)?; + self._add_edge(*edge_target, *edge_source, weight)?; } } Ok(()) From fd924ba099df22ab3189eb8eda14e4ec6606bc02 Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Fri, 12 May 2023 09:24:01 -0400 Subject: [PATCH 10/10] Fix lint --- src/digraph.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/digraph.rs b/src/digraph.rs index 0af1a5c27..25d0af74f 100644 --- a/src/digraph.rs +++ b/src/digraph.rs @@ -2735,7 +2735,7 @@ impl PyDiGraph { .edge_references() .map(|edge| ([edge.source(), edge.target()], edge.id())) .collect(); - for ([edge_source, edge_target], edge_index) in edges.iter() { + for ([edge_source, edge_target], edge_index) in edges.iter() { if !edges.contains_key(&[*edge_target, *edge_source]) { let forward_weight = self.graph.edge_weight(*edge_index).unwrap(); let weight: PyObject = match edge_payload_fn.as_ref() {