From d841c04ef75f528f18d1486bec5037224618a2c3 Mon Sep 17 00:00:00 2001 From: Rick Wierenga Date: Thu, 14 Nov 2024 17:25:14 -0800 Subject: [PATCH] stricter name collision checking --- pylabrobot/resources/deck.py | 11 ++--------- pylabrobot/resources/resource.py | 19 +++++++++++++++++++ pylabrobot/resources/resource_tests.py | 16 ++++++++++++++++ pylabrobot/resources/utils_tests.py | 4 ++-- 4 files changed, 39 insertions(+), 11 deletions(-) diff --git a/pylabrobot/resources/deck.py b/pylabrobot/resources/deck.py index b118a05705..2f78f13b93 100644 --- a/pylabrobot/resources/deck.py +++ b/pylabrobot/resources/deck.py @@ -39,7 +39,6 @@ def __init__( self.location = origin self.resources: Dict[str, Resource] = {} - 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 +48,10 @@ 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_naming_conflicts(self, resource: Resource): + """overwrite for speed""" 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 _register_resource(self, resource: Resource): """Recursively assign the given resource and all child resources to the `self.resources` diff --git a/pylabrobot/resources/resource.py b/pylabrobot/resources/resource.py index af6309090d..d0bfe51063 100644 --- a/pylabrobot/resources/resource.py +++ b/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. diff --git a/pylabrobot/resources/resource_tests.py b/pylabrobot/resources/resource_tests.py index f5bf83a991..1529acd1be 100644 --- a/pylabrobot/resources/resource_tests.py +++ b/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( diff --git a/pylabrobot/resources/utils_tests.py b/pylabrobot/resources/utils_tests.py index e3439fc25e..0a5dca80ab 100644 --- a/pylabrobot/resources/utils_tests.py +++ b/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]