Check if nodes exist on add_edge methods (Qiskit#862)

* Add tests from the example * Fix bug * Fix tests * Add release note * Update release note * Apply suggestions from code review Co-authored-by: Matthew Treinish <mtreinish@kortar.org> * Fix docs to work with Sphinx Theme 1.11 (Qiskit#867) * Fix docs to work with Sphinx Theme 1.11 * Update docs/source/_templates/sidebar.html Co-authored-by: Matthew Treinish <mtreinish@kortar.org> * Turn off CI for forks (Qiskit#868) Co-authored-by: Matthew Treinish <mtreinish@kortar.org> * Fix pickle/deepcopy not preserve original edge indices (Qiskit#589) * fix issue Qiskit#585 that pickling graph & digraph do not preserve original edge index * fix clippy lints - collapsible_else_if * Simplify logic in __setstate__ * Add release note * Fix lint --------- Co-authored-by: Matthew Treinish <mtreinish@kortar.org> * Bump serde from 1.0.160 to 1.0.162 (Qiskit#863) Bumps [serde](https://github.com/serde-rs/serde) from 1.0.160 to 1.0.162. - [Release notes](https://github.com/serde-rs/serde/releases) - [Commits](serde-rs/serde@v1.0.160...1.0.162) --- updated-dependencies: - dependency-name: serde dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Matthew Treinish <mtreinish@kortar.org> * Add reverse inplace function for digraph (Qiskit#853) * added a reverse_inplace function in digraph, the function reverses the direction of the edges in the digraph implemented by switching the indices of the nodes in an edge. * added python tests for the reverse_inplace function. testing a simple case and a case for a large graph. * ran rust fmt and clippy, also added more detailed documentation * rename reverse_inplace to reverse * change excepts to unwraps (If this fails is because of PyO3. It panics and there is not much point in printing a message) * added tests for empty graph and graph with node removed in the middle * added interface signature for IDEs * ran cargo fmt * Fix doc syntax --------- Co-authored-by: Matthew Treinish <mtreinish@kortar.org> * Bump serde from 1.0.162 to 1.0.163 (Qiskit#869) Bumps [serde](https://github.com/serde-rs/serde) from 1.0.162 to 1.0.163. - [Release notes](https://github.com/serde-rs/serde/releases) - [Commits](serde-rs/serde@v1.0.162...v1.0.163) --- updated-dependencies: - dependency-name: serde dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Extend fixes to add_edges_from and add_edges_from_no_data * Lower amount of nodes in test --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: Matthew Treinish <mtreinish@kortar.org> Co-authored-by: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com> Co-authored-by: Binh Vu <binh-vu@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: matanco64 <38103422+matanco64@users.noreply.github.com>
IvanIsCoding · May 26, 2023 · 71890f2 · 71890f2
1 parent 74743e2
commit 71890f2
Show file tree

Hide file tree

Showing 7 changed files with 63 additions and 22 deletions.
diff --git a/releasenotes/notes/handle-non-existent-edge-15d70cfe60c89ac2.yaml b/releasenotes/notes/handle-non-existent-edge-15d70cfe60c89ac2.yaml
@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    :meth:`rustworkx.PyGraph.add_edge` and :meth:`rustworkx.PyDiGraph.add_edge` and now raises an 
+    ``IndexError`` when one of the nodes does not exist in the graph. Previously, it caused the Python 
+    interpreter to exit with a ``PanicException``
diff --git a/src/connectivity/mod.rs b/src/connectivity/mod.rs
@@ -398,7 +398,7 @@ pub fn graph_complement(py: Python, graph: &graph::PyGraph) -> PyResult<graph::P
                     || !complement_graph.has_edge(node_a.index(), node_b.index()))
             {
                 // avoid creating parallel edges in multigraph
-                complement_graph.add_edge(node_a.index(), node_b.index(), py.None());
+                complement_graph.graph.add_edge(node_a, node_b, py.None());
             }
         }
     }

diff --git a/src/digraph.rs b/src/digraph.rs
@@ -1083,6 +1083,11 @@ impl PyDiGraph {
     pub fn add_edge(&mut self, parent: usize, child: usize, edge: PyObject) -> PyResult<usize> {
         let p_index = NodeIndex::new(parent);
         let c_index = NodeIndex::new(child);
+        if !self.graph.contains_node(p_index) || !self.graph.contains_node(c_index) {
+            return Err(PyIndexError::new_err(
+                "One of the endpoints of the edge does not exist in graph",
+            ));
+        }
         let out_index = self._add_edge(p_index, c_index, edge)?;
         Ok(out_index)
     }
@@ -1103,9 +1108,7 @@ impl PyDiGraph {
     ) -> PyResult<Vec<usize>> {
         let mut out_list: Vec<usize> = Vec::with_capacity(obj_list.len());
         for obj in obj_list {
-            let p_index = NodeIndex::new(obj.0);
-            let c_index = NodeIndex::new(obj.1);
-            let edge = self._add_edge(p_index, c_index, obj.2)?;
+            let edge = self.add_edge(obj.0, obj.1, obj.2)?;
             out_list.push(edge);
         }
         Ok(out_list)
@@ -1129,9 +1132,7 @@ impl PyDiGraph {
     ) -> PyResult<Vec<usize>> {
         let mut out_list: Vec<usize> = Vec::with_capacity(obj_list.len());
         for obj in obj_list {
-            let p_index = NodeIndex::new(obj.0);
-            let c_index = NodeIndex::new(obj.1);
-            let edge = self._add_edge(p_index, c_index, py.None())?;
+            let edge = self.add_edge(obj.0, obj.1, py.None())?;
             out_list.push(edge);
         }
         Ok(out_list)

diff --git a/src/graph.rs b/src/graph.rs
@@ -847,10 +847,15 @@ impl PyGraph {
     ///     of an existing edge with ``multigraph=False``) edge.
     /// :rtype: int
     #[pyo3(text_signature = "(self, node_a, node_b, edge, /)")]
-    pub fn add_edge(&mut self, node_a: usize, node_b: usize, edge: PyObject) -> usize {
+    pub fn add_edge(&mut self, node_a: usize, node_b: usize, edge: PyObject) -> PyResult<usize> {
         let p_index = NodeIndex::new(node_a);
         let c_index = NodeIndex::new(node_b);
-        self._add_edge(p_index, c_index, edge)
+        if !self.graph.contains_node(p_index) || !self.graph.contains_node(c_index) {
+            return Err(PyIndexError::new_err(
+                "One of the endpoints of the edge does not exist in graph",
+            ));
+        }
+        Ok(self._add_edge(p_index, c_index, edge))
     }
 
     /// Add new edges to the graph.
@@ -869,14 +874,15 @@ impl PyGraph {
     /// :returns: A list of int indices of the newly created edges
     /// :rtype: list
     #[pyo3(text_signature = "(self, obj_list, /)")]
-    pub fn add_edges_from(&mut self, obj_list: Vec<(usize, usize, PyObject)>) -> EdgeIndices {
+    pub fn add_edges_from(
+        &mut self,
+        obj_list: Vec<(usize, usize, PyObject)>,
+    ) -> PyResult<EdgeIndices> {
         let mut out_list: Vec<usize> = Vec::with_capacity(obj_list.len());
         for obj in obj_list {
-            let p_index = NodeIndex::new(obj.0);
-            let c_index = NodeIndex::new(obj.1);
-            out_list.push(self._add_edge(p_index, c_index, obj.2));
+            out_list.push(self.add_edge(obj.0, obj.1, obj.2)?);
         }
-        EdgeIndices { edges: out_list }
+        Ok(EdgeIndices { edges: out_list })
     }
 
     /// Add new edges to the graph without python data.
@@ -898,14 +904,12 @@ impl PyGraph {
         &mut self,
         py: Python,
         obj_list: Vec<(usize, usize)>,
-    ) -> EdgeIndices {
+    ) -> PyResult<EdgeIndices> {
         let mut out_list: Vec<usize> = Vec::with_capacity(obj_list.len());
         for obj in obj_list {
-            let p_index = NodeIndex::new(obj.0);
-            let c_index = NodeIndex::new(obj.1);
-            out_list.push(self._add_edge(p_index, c_index, py.None()));
+            out_list.push(self.add_edge(obj.0, obj.1, py.None())?);
         }
-        EdgeIndices { edges: out_list }
+        Ok(EdgeIndices { edges: out_list })
     }
 
     /// Extend graph from an edge list
@@ -1703,7 +1707,7 @@ impl PyGraph {
         }
 
         for (source, weight) in edges {
-            self.add_edge(source.index(), node_index.index(), weight);
+            self.add_edge(source.index(), node_index.index(), weight)?;
         }
 
         Ok(node_index.index())

diff --git a/src/tree.rs b/src/tree.rs
@@ -130,7 +130,7 @@ pub fn minimum_spanning_tree(
         .edges
         .iter()
     {
-        spanning_tree.add_edge(edge.0, edge.1, edge.2.clone_ref(py));
+        spanning_tree.add_edge(edge.0, edge.1, edge.2.clone_ref(py))?;
     }
 
     Ok(spanning_tree)

diff --git a/tests/rustworkx_tests/digraph/test_edges.py b/tests/rustworkx_tests/digraph/test_edges.py
@@ -963,6 +963,21 @@ def test_extend_from_weighted_edge_list(self):
         self.assertEqual(len(graph), 4)
         self.assertEqual(["a", "b", "c", "d", "e"], graph.edges())
 
+    def test_add_edge_non_existent(self):
+        g = rustworkx.PyDiGraph()
+        with self.assertRaises(IndexError):
+            g.add_edge(2, 3, None)
+
+    def test_add_edges_from_non_existent(self):
+        g = rustworkx.PyDiGraph()
+        with self.assertRaises(IndexError):
+            g.add_edges_from([(2, 3, 5)])
+
+    def test_add_edges_from_no_data_non_existent(self):
+        g = rustworkx.PyDiGraph()
+        with self.assertRaises(IndexError):
+            g.add_edges_from_no_data([(2, 3)])
+
     def test_reverse_graph(self):
         graph = rustworkx.PyDiGraph()
         graph.add_nodes_from([i for i in range(4)])
@@ -978,7 +993,7 @@ def test_reverse_graph(self):
         self.assertEqual([(1, 0), (2, 1), (2, 0), (3, 2), (3, 0)], graph.edge_list())
 
     def test_reverse_large_graph(self):
-        LARGE_AMOUNT_OF_NODES = 10000000
+        LARGE_AMOUNT_OF_NODES = 1000000
 
         graph = rustworkx.PyDiGraph()
         graph.add_nodes_from(range(LARGE_AMOUNT_OF_NODES))

diff --git a/tests/rustworkx_tests/graph/test_edges.py b/tests/rustworkx_tests/graph/test_edges.py
@@ -817,3 +817,18 @@ def test_extend_from_weighted_edge_list(self):
         graph.extend_from_weighted_edge_list(edge_list)
         self.assertEqual(len(graph), 4)
         self.assertEqual(["a", "b", "c", "d", "e"], graph.edges())
+
+    def test_add_edge_non_existent(self):
+        g = rustworkx.PyGraph()
+        with self.assertRaises(IndexError):
+            g.add_edge(2, 3, None)
+
+    def test_add_edges_from_non_existent(self):
+        g = rustworkx.PyGraph()
+        with self.assertRaises(IndexError):
+            g.add_edges_from([(2, 3, 5)])
+
+    def test_add_edges_from_no_data_non_existent(self):
+        g = rustworkx.PyGraph()
+        with self.assertRaises(IndexError):
+            g.add_edges_from_no_data([(2, 3)])
-Original file line number
+Diff line change
@@ Expand Up / @@ -130,7 +130,7 @@ pub fn minimum_spanning_tree( @@
             .edges
             .iter()
         {
-            spanning_tree.add_edge(edge.0, edge.1, edge.2.clone_ref(py));
+            spanning_tree.add_edge(edge.0, edge.1, edge.2.clone_ref(py))?;
         }
         Ok(spanning_tree)
@@ Expand Down @@