Merge pull request #40 from jaimervq/39_check_for_cycles

[#39] Remove possibility to create a cycle in the graph

.github/workflows/tests.yml

@@ -18,10 +18,10 @@ jobs:
 
     steps:
 
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
 
       - name: Set up Python ${{ matrix.python-version }}
-        uses: actions/setup-python@v4
+        uses: actions/setup-python@v5
         with:
           python-version: ${{ matrix.python-version }}
 

analytics/analytics.py

@@ -31,11 +31,11 @@
 # -------------------------------- CLIENT -------------------------------- #
 mongo_client = None
 try:
-    LOGGER.info(f"Connecting to MongoDB for analytics, username: {DB_USERNAME}")
     mongo_client = MongoClient(CONNECTION_STRING)
     LOGGER.debug(mongo_client.admin.command("ping"))
+    LOGGER.info(f"Connected to MongoDB for analytics, username: {DB_USERNAME}")
 except Exception:
-    LOGGER.warning("Could not connect to MongoDB!")
+    LOGGER.debug("No statistics will be submitted")
 
 
 # -------------------------------- METHODS -------------------------------- #

graphic/graphic_node.py

@@ -997,8 +997,9 @@ def connect_graphic_attr(self, other_graphic_attr, check_logic=True):
         Returns: bool, whether the attribute could be connected
         """
         can_connect = True
+        reason = ""
         if check_logic:
-            can_connect = self.logic_attribute.connect_to_other(
+            can_connect, reason = self.logic_attribute.connect_to_other(
                 other_graphic_attr.logic_attribute
             )
         if can_connect:
@@ -1012,7 +1013,7 @@ def connect_graphic_attr(self, other_graphic_attr, check_logic=True):
         self.show_connected_status()
         other_graphic_attr.show_connected_status()
 
-        return can_connect
+        return can_connect, reason
 
     def clear_connections(self):
         connected_g_attrs = list(self.connected_graphic_attrs)

graphic/graphic_scene.py

@@ -327,16 +327,19 @@ def connect_graphic_attrs(
                     self.removeItem(item)
 
         # Connect
-        if graphic_attr_1.connect_graphic_attr(graphic_attr_2, check_logic):
+        can_connect, reason = graphic_attr_1.connect_graphic_attr(
+            graphic_attr_2, check_logic
+        )
+        if can_connect:
             self.in_screen_feedback.emit(
-                "Connected graphic attributes!",
+                reason,
                 logging.DEBUG,
             )
             self.draw_valid_line(graphic_attr_1, graphic_attr_2)
         else:
             self.in_screen_feedback.emit(
-                "Cannot connect those attributes",
-                logging.ERROR,
+                reason,
+                logging.WARNING,
             )
 
     def draw_valid_line(self, graphic_attr_1, graphic_attr_2):

logic/logic_node.py

@@ -212,6 +212,25 @@ def out_connected_nodes(self):
                 connected_nodes.add(connected_attr.parent_node)
         return connected_nodes
 
+    def in_connected_nodes_recursive(self):
+        in_connected_nodes = list()
+
+        in_immediate_nodes = set()
+        for attr in self.get_input_attrs():
+            for connected_attr in attr.connected_attributes:
+                in_immediate_nodes.add(connected_attr.parent_node)
+
+        in_immediate_nodes_list = list(in_immediate_nodes)
+        in_connected_nodes += in_immediate_nodes_list
+        for node in in_immediate_nodes_list:
+            in_connected_nodes += node.in_connected_nodes_recursive()
+
+        return in_connected_nodes
+
+    def check_cycles(self, node_to_check):
+        in_connected_nodes = self.in_connected_nodes_recursive()
+        return node_to_check in in_connected_nodes
+
     # PROPERTIES ----------------------
     @property
     def full_name(self):
@@ -898,32 +917,42 @@ def connect_to_other(self, other_attribute: GeneralLogicAttribute) -> bool:
             other_attribute (GeneralLogicAttribute)
 
         Returns:
-            bool: whether or not the connection could be done
+            tuple(bool, str): whether or not the connection could be done, log of the reason
         """
+        # Checks -------------------------
+        # Cycles check
+        if self.parent_node.check_cycles(
+            other_attribute.parent_node
+        ) or other_attribute.parent_node.check_cycles(self.parent_node):
+            connection_warning = "Cannot connect, cycle detected!"
+            LOGGER.warning(connection_warning)
+            return (False, connection_warning)
+
+        # Different nodes check
         if not self.parent_node != other_attribute.parent_node:
-            LOGGER.warning(
-                "Cannot connect {} and {}, both same node {}".format(
-                    self.dot_name,
-                    other_attribute.dot_name,
-                    self.parent_node.full_name,
-                )
+            connection_warning = "Cannot connect {} and {}, both same node {}".format(
+                self.dot_name,
+                other_attribute.dot_name,
+                self.parent_node.full_name,
             )
-            return False
+            LOGGER.warning(connection_warning)
+            return (False, connection_warning)
 
+        # IN - OUT check
         if not self.connector_type != other_attribute.connector_type:
-            LOGGER.warning(
-                "Cannot connect {} and {}, both are {}".format(
-                    self.dot_name, other_attribute.dot_name, self.connector_type
-                )
+            connection_warning = "Cannot connect {} and {}, both are {}".format(
+                self.dot_name, other_attribute.dot_name, self.connector_type
             )
-            return False
+            LOGGER.warning(connection_warning)
+            return (False, "Cannot connect both are {}".format(self.connector_type))
 
+        # Datatype check
         connection_direction = "->"
         if self.connector_type == constants.INPUT:
             connection_direction = "<-"
-        if not self.data_type == other_attribute.data_type:
+        if self.data_type != other_attribute.data_type:
             if object not in [self.data_type, other_attribute.data_type]:
-                LOGGER.warning(
+                connection_warning = (
                     "Cannot connect {} {} {}, different datatypes {}{}{}".format(
                         self.dot_name,
                         connection_direction,
@@ -933,22 +962,30 @@ def connect_to_other(self, other_attribute: GeneralLogicAttribute) -> bool:
                         other_attribute.get_datatype_str(),
                     )
                 )
-                return False
+                LOGGER.warning(connection_warning)
+                return (
+                    False,
+                    "Cannot connect, different datatypes {}{}{}".format(
+                        self.get_datatype_str(),
+                        connection_direction,
+                        other_attribute.get_datatype_str(),
+                    ),
+                )
 
+        # Connection -------------------------
         if self.connector_type == constants.OUTPUT:
             self.connected_attributes.add(other_attribute)
             other_attribute.connected_attributes = {self}
         else:
             self.connected_attributes = {other_attribute}
             other_attribute.connected_attributes.add(self)
 
-        LOGGER.info(
-            "Connected {} {} {}".format(
-                self.dot_name, connection_direction, other_attribute.dot_name
-            )
+        connection_log = "Connected {} {} {}".format(
+            self.dot_name, connection_direction, other_attribute.dot_name
         )
+        LOGGER.info(connection_log)
 
-        return True
+        return (True, connection_log)
 
     def disconnect_from_other(self, other_attribute: GeneralLogicAttribute):
         """

test/test_node.py

@@ -49,10 +49,26 @@ def test_connection(self):
 
         n_1 = file_reading_nodes.JsonToDict()
         n_2 = general_nodes.GetDictKey()
-        self.assertTrue(n_1.connect_attribute("out_dict", n_2, "in_dict"))
-        self.assertFalse(n_1["out_dict"].connect_to_other(n_2["key"]))
-        self.assertFalse(n_2["in_dict"].connect_to_other(n_2["key"]))
-        self.assertFalse(n_2["key"].connect_to_other(n_1["out_dict"]))
+        self.assertTrue(n_1.connect_attribute("out_dict", n_2, "in_dict")[0])
+        self.assertFalse(n_1["out_dict"].connect_to_other(n_2["key"])[0])
+        self.assertFalse(n_2["in_dict"].connect_to_other(n_2["key"])[0])
+        self.assertFalse(n_2["key"].connect_to_other(n_1["out_dict"])[0])
+
+    def test_connection_cycle(self):
+        """
+        Test connecting some node attributes.
+        """
+        utils.print_test_header("test_connection_cycle")
+
+        n_1 = debug_nodes.EmptyNode()
+        n_2 = debug_nodes.EmptyNode()
+        n_3 = debug_nodes.EmptyNode()
+        n_1.connect_attribute(constants.COMPLETED, n_2, constants.START)
+        n_2.connect_attribute(constants.COMPLETED, n_3, constants.START)
+        connection_status, _ = n_3.connect_attribute(
+            constants.COMPLETED, n_1, constants.START
+        )
+        self.assertFalse(connection_status)
 
     def test_getting_internal_dict(self):
         """