From ac434db8c6dffabc22be1a0ec3461ca658a9e777 Mon Sep 17 00:00:00 2001 From: Mark Wolfman Date: Wed, 16 Oct 2024 14:42:52 -0500 Subject: [PATCH 1/2] Updated handling of duplicate devices. Previously, the old device would be retained and the new device would be ignored. Now, the new device overwrites the old device. Also, the warning for a duplicate name is only issued for the top level devices in the device hierarchy. --- src/ophydregistry/registry.py | 20 +++++++++++++------ .../tests/test_instrument_registry.py | 15 ++++++++++---- 2 files changed, 25 insertions(+), 10 deletions(-) diff --git a/src/ophydregistry/registry.py b/src/ophydregistry/registry.py index e40a94a..b076d5a 100644 --- a/src/ophydregistry/registry.py +++ b/src/ophydregistry/registry.py @@ -519,7 +519,7 @@ def __new__wrapper(self, cls, *args, **kwargs): return obj def register( - self, component: ophydobj.OphydObject, labels: Optional[Sequence] = None + self, component: ophydobj.OphydObject, labels: Optional[Sequence] = None, warn_duplicates=True ) -> ophydobj.OphydObject: """Register a device, component, etc so that it can be retrieved later. @@ -534,6 +534,9 @@ def register( labels Device labels to use for registration. If `None` (default), the devices *_ophyd_labels_* parameter will be used. + warn_duplicates + If true, a warning will be issued if this device is + overwriting a previous device with the same name. """ # Determine how to register the device @@ -552,7 +555,8 @@ def register( # Ignore any instances with the same name as a previous component # (Needed for some sub-components that are just readback # values of the parent) - # Check that we're not adding a duplicate component name + # Check if we're adding a duplicate component name + is_duplicate = False if name in self._objects_by_name.keys(): old_obj = self._objects_by_name[name] is_readback = component in [ @@ -563,11 +567,15 @@ def register( if is_readback: msg = f"Ignoring readback with duplicate name: '{name}'" log.debug(msg) + return component else: msg = f"Ignoring component with duplicate name: '{name}'" - log.warning(msg) - warnings.warn(msg) - return component + is_duplicate = True + if warn_duplicates: + log.warning(msg) + warnings.warn(msg) + else: + log.debug(msg) # Register this component log.debug(f"Registering {name}") # Create a set for this device name if it doesn't exist @@ -599,5 +607,5 @@ def register( else: sub_signals = [] for cpt_name, cpt in sub_signals: - self.register(cpt) + self.register(cpt, warn_duplicates=not is_duplicate and warn_duplicates) return component diff --git a/src/ophydregistry/tests/test_instrument_registry.py b/src/ophydregistry/tests/test_instrument_registry.py index 9bc0ae6..8f8e05e 100644 --- a/src/ophydregistry/tests/test_instrument_registry.py +++ b/src/ophydregistry/tests/test_instrument_registry.py @@ -390,11 +390,13 @@ def test_getitem(registry): def test_duplicate_device(caplog, registry): """Check that a device doesn't get added twice.""" - motor = sim.motor + # Two devices with the same name + motor1 = sim.instantiate_fake_device(EpicsMotor, prefix="", name="motor") + motor2 = sim.instantiate_fake_device(EpicsMotor, prefix="", name="motor") # Set up logging so that we can know what caplog.clear() with caplog.at_level(logging.DEBUG): - registry.register(motor) + registry.register(motor1) # Check for the edge case where motor and motor.user_readback have the same name assert "Ignoring component with duplicate name" not in caplog.text assert "Ignoring readback with duplicate name" in caplog.text @@ -402,9 +404,14 @@ def test_duplicate_device(caplog, registry): caplog.clear() with caplog.at_level(logging.WARNING): with pytest.warns(UserWarning): - registry.register(motor) + registry.register(motor2) # Check for the edge case where motor and motor.user_readback have the same name assert "Ignoring component with duplicate name" in caplog.text + print(caplog.text) + # Check that the warning is only issued for the top-level device, not all its children + assert "motor_user_setpoint" not in caplog.text + # Check that the correct second device is the one that wound up in the registry + assert registry['motor'] is motor2 def test_delete_by_name(registry): @@ -472,7 +479,7 @@ def test_weak_references(): """ motor = sim.SynAxis(name="weak_motor", labels={"motors"}) - registry = Registry(keep_references=False) + registry = Registry(keep_references=False, auto_register=False) registry.register(motor) # Can we still find the object if the test has a reference? assert registry.find("weak_motor") is motor From 626bbd38670da48d9438e41f0e7d277a14e82d2d Mon Sep 17 00:00:00 2001 From: Mark Wolfman Date: Wed, 16 Oct 2024 14:46:31 -0500 Subject: [PATCH 2/2] Black, isort, and flake8. --- src/ophydregistry/registry.py | 5 ++++- src/ophydregistry/tests/test_instrument_registry.py | 8 +++++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/ophydregistry/registry.py b/src/ophydregistry/registry.py index b076d5a..379544c 100644 --- a/src/ophydregistry/registry.py +++ b/src/ophydregistry/registry.py @@ -519,7 +519,10 @@ def __new__wrapper(self, cls, *args, **kwargs): return obj def register( - self, component: ophydobj.OphydObject, labels: Optional[Sequence] = None, warn_duplicates=True + self, + component: ophydobj.OphydObject, + labels: Optional[Sequence] = None, + warn_duplicates=True, ) -> ophydobj.OphydObject: """Register a device, component, etc so that it can be retrieved later. diff --git a/src/ophydregistry/tests/test_instrument_registry.py b/src/ophydregistry/tests/test_instrument_registry.py index 8f8e05e..09cc621 100644 --- a/src/ophydregistry/tests/test_instrument_registry.py +++ b/src/ophydregistry/tests/test_instrument_registry.py @@ -6,7 +6,8 @@ import pytest from ophyd import Device, EpicsMotor, sim -from ophyd_async.core import Device as AsyncDevice, soft_signal_rw +from ophyd_async.core import Device as AsyncDevice +from ophyd_async.core import soft_signal_rw from ophydregistry import ComponentNotFound, MultipleComponentsFound, Registry @@ -146,11 +147,12 @@ def test_find_component(registry): def test_find_async_children(registry): """Check that the child components of an async device get registered.""" + class MyDevice(AsyncDevice): def __init__(self, name): self.signal = soft_signal_rw() super().__init__(name=name) - + device = MyDevice(name="m1") registry.register(device) assert registry.find(device.signal.name) is device.signal @@ -411,7 +413,7 @@ def test_duplicate_device(caplog, registry): # Check that the warning is only issued for the top-level device, not all its children assert "motor_user_setpoint" not in caplog.text # Check that the correct second device is the one that wound up in the registry - assert registry['motor'] is motor2 + assert registry["motor"] is motor2 def test_delete_by_name(registry):