Bug: Possible to add Edges forward and backwards

cai_causal_graph/causal_graph.py

@@ -741,6 +741,10 @@ def _set_edge(self, edge: Edge, validate: bool = True):
             edge.destination
         )
 
+        # raise an error if reverse edge from destination to source has already been defined
+        if self._edges_by_source[destination].get(source) is not None:
+            raise CausalGraphErrors.ReverseEdgeExistsError()
+
         self._edges_by_source[source][destination] = edge
         self._edges_by_destination[destination][source] = edge
 

cai_causal_graph/exceptions.py

@@ -45,6 +45,9 @@ class NodeInvalidError(Exception):
     class EdgeExistsError(Exception):
         """Error raised when edge already exists in the graph."""
 
+    class ReverseEdgeExistsError(Exception):
+        """Error raised when the reverse edge already exists in the graph."""
+
     class EdgeDoesNotExistError(Exception):
         """Error raised when edge does not exist in the graph."""
 

changelog.md

@@ -1,5 +1,12 @@
 # Changelog
 
+## NEXT
+
+- Fixed a bug in the `cai_causal_graph.causal_graph.CausalGraph.add_edge` method that would allow the addition of an
+  edge between `b` and `a` when the reversed edge, i.e. between `a` and `b` was already specified.
+  - Added the `CausalGraphError.ReverseEdgeExistsError` exception, which distinguishes errors arising from the
+    introduction of cycles or reverse edges.
+
 ## 0.5.4
 
 - Improved the speed of the `cai_causal_graph.causal_graph.CausalGraph.get_descendant_graph` and

tests/test_causal_graph.py

@@ -631,7 +631,7 @@ def test_add_edges_from_single_path(self):
 
         # check conflicting paths raises
         causal_graph.add_edges_from_paths(['a3', 'b', 'c'])
-        with self.assertRaises(CausalGraphErrors.CyclicConnectionError):
+        with self.assertRaises(CausalGraphErrors.ReverseEdgeExistsError):
             causal_graph.add_edges_from_paths(['c', 'b', 'z'])
 
     def test_add_edges_from_multiple_paths(self):
@@ -645,7 +645,7 @@ def test_add_edges_from_multiple_paths(self):
         )
 
         # check conflicting paths raises
-        with self.assertRaises(CausalGraphErrors.CyclicConnectionError):
+        with self.assertRaises(CausalGraphErrors.ReverseEdgeExistsError):
             causal_graph.add_edges_from_paths([['a3', 'b', 'c'], ['c', 'b', 'z']])
 
         # check that invalid paths raises (mix of paths and strings)
@@ -1412,3 +1412,22 @@ def test_node_repr(self):
         self.assertEqual(tscg['b'], tscg.get_node('b'))
         self.assertEqual(repr(tscg['a']), 'TimeSeriesNode("a")')
         self.assertEqual(repr(tscg['b']), 'TimeSeriesNode("b", type="continuous")')
+
+    def test_add_to_undirected_edges(self):
+        # adding an edge dest-src to an existing edge src-dest should raise an error
+
+        cg = CausalGraph()
+        cg.add_edge('a', 'b', edge_type=EdgeType.UNDIRECTED_EDGE)
+
+        with self.assertRaises(CausalGraphErrors.ReverseEdgeExistsError):
+            cg.add_edge('b', 'a')
+        with self.assertRaises(CausalGraphErrors.ReverseEdgeExistsError):
+            cg.add_edge('b', 'a', edge_type=EdgeType.UNDIRECTED_EDGE)
+        with self.assertRaises(CausalGraphErrors.ReverseEdgeExistsError):
+            cg.add_edge('b', 'a', edge_type=EdgeType.BIDIRECTED_EDGE)
+        with self.assertRaises(CausalGraphErrors.ReverseEdgeExistsError):
+            cg.add_edge('b', 'a', edge_type=EdgeType.UNKNOWN_EDGE)
+        with self.assertRaises(CausalGraphErrors.ReverseEdgeExistsError):
+            cg.add_edge('b', 'a', edge_type=EdgeType.UNKNOWN_DIRECTED_EDGE)
+        with self.assertRaises(CausalGraphErrors.ReverseEdgeExistsError):
+            cg.add_edge('b', 'a', edge_type=EdgeType.UNKNOWN_UNDIRECTED_EDGE)