stricter name collision checking

pylabrobot/resources/deck.py

@@ -39,7 +39,7 @@ def __init__(
     self.location = origin
     self.resources: Dict[str, Resource] = {}
 
-    self.register_will_assign_resource_callback(self._check_name_exists)
+    # self.register_will_assign_resource_callback(self._check_name_exists)
     self.register_did_assign_resource_callback(self._register_resource)
     self.register_did_unassign_resource_callback(self._deregister_resource)
 
@@ -49,16 +49,24 @@ def serialize(self) -> dict:
     del super_serialized["model"]  # deck's don't typically have a model
     return super_serialized
 
-  def _check_name_exists(self, resource: Resource):
-    """Callback called before a resource is assigned to the deck. (will_assign_resource_callback)
-    Raises a ValueError if the resource name already exists. This method is recursive, and
-    will also check children of the resource that is to be assigned.
-    """
+  # def _check_name_exists(self, resource: Resource):
+  #   """Callback called before a resource is assigned to the deck. (will_assign_resource_callback)
+  #   Raises a ValueError if the resource name already exists. This method is recursive, and
+  #   will also check children of the resource that is to be assigned.
+  #   """
+
+  #   if self.has_resource(resource.name):
+  #     raise ValueError(f"Resource '{resource.name}' already assigned to deck")
+  #   for child in resource.children:
+  #     self._check_name_exists(child)
 
+  def _check_naming_conflicts(self, resource: Resource):
+    """overwrite for speed"""
+    # print(0/0)
     if self.has_resource(resource.name):
       raise ValueError(f"Resource '{resource.name}' already assigned to deck")
-    for child in resource.children:
-      self._check_name_exists(child)
+    # for child in resource.children:
+    #   self._check_naming_conflicts(child)
 
   def _register_resource(self, resource: Resource):
     """Recursively assign the given resource and all child resources to the `self.resources`

pylabrobot/resources/resource.py

@@ -288,6 +288,7 @@ def assign_child_resource(
 
     # Check for unsupported resource assignment operations
     self._check_assignment(resource=resource, reassign=reassign)
+    self.get_root()._check_naming_conflicts(resource=resource)
 
     # Call "will assign" callbacks
     for callback in self._will_assign_resource_callbacks:
@@ -360,6 +361,24 @@ def _check_assignment(self, resource: Resource, reassign: bool = True):
       msg = " ".join(msgs)
       raise ValueError(msg)
 
+  def get_root(self) -> Resource:
+    """Get the root of the resource tree."""
+    if self.parent is None:
+      return self
+    return self.parent.get_root()
+
+  def _check_naming_conflicts(self, resource: Resource):
+    """Recursively check for naming conflicts in the resource tree."""
+    if resource.name == self.name:
+      raise ValueError(f"Resource with name '{resource.name}' already exists in the tree.")
+
+    # check if the name of the resource we are currently checking already exists in this subtree
+    for child in self.children:
+      child._check_naming_conflicts(resource)
+    # check if the name of any of the children of the resource already exists in this subtree
+    for child in resource.children:
+      self._check_naming_conflicts(child)
+
   def unassign_child_resource(self, resource: Resource):
     """Unassign a child resource from this resource.
 

pylabrobot/resources/resource_tests.py

@@ -85,6 +85,22 @@ def test_assign_name_taken(self):
       other_child = Resource("child", size_x=5, size_y=5, size_z=5)
       deck.assign_child_resource(other_child, location=Coordinate(5, 5, 5))
 
+  def test_assign_name_exists_in_tree(self):
+    root = Resource("root", size_x=10, size_y=10, size_z=10)
+    child1 = Resource("child", size_x=5, size_y=5, size_z=5)
+    root.assign_child_resource(child1, location=Coordinate(5, 5, 5))
+    child2 = Resource("child", size_x=5, size_y=5, size_z=5)
+    with self.assertRaises(ValueError):
+      root.assign_child_resource(child2, location=Coordinate(5, 5, 5))
+
+    grandchild1 = Resource("grandchild", size_x=5, size_y=5, size_z=5)
+    child1.assign_child_resource(grandchild1, location=Coordinate(5, 5, 5))
+    child3 = Resource("child3", size_x=5, size_y=5, size_z=5)
+    root.assign_child_resource(child3, location=Coordinate(5, 5, 5))
+    grandchild2 = Resource("grandchild", size_x=5, size_y=5, size_z=5)
+    with self.assertRaises(ValueError):
+      root.assign_child_resource(grandchild2, location=Coordinate(5, 5, 5))
+
   def test_get_anchor(self):
     resource = Resource("test", size_x=12, size_y=12, size_z=12)
     self.assertEqual(

pylabrobot/resources/utils_tests.py

@@ -21,8 +21,8 @@ def test_query():
 
 def test_query_with_type():
   root = Resource(name="root", size_x=10, size_y=10, size_z=10)
-  well1 = Well(name="well", size_x=3, size_y=3, size_z=3)
-  well2 = Well(name="well", size_x=3, size_y=3, size_z=3)
+  well1 = Well(name="well1", size_x=3, size_y=3, size_z=3)
+  well2 = Well(name="well2", size_x=3, size_y=3, size_z=3)
   root.assign_child_resource(well1, location=Coordinate(6, 1, 0))
   root.assign_child_resource(well2, location=Coordinate(6, 6, 0))
   assert query(root, Well) == [well1, well2]