Description
At the moment, Device naming happens via Device.set_name
. This is called:
- in
Device.__init__(name)
at construction - after construction by the
init_devices
ordodal.common.beamlines.beamline_utils.device_factory
This leads to some odd edge conditions:
class Base(Device):
def __init__(self, name=""):
self.signal1 = soft_signal_rw()
super().__init__(name=name)
class Derived(Base):
def __init__(self, name=""):
super().__init__(name=name)
self.signal2 = soft_signal_rw(name="blah")
# Naming at init only names those that are present when the superclass init is called
derived1 = Derived(name="derived")
assert derived1.signal1.name == "derived-signal1"
assert derived1.signal2.name == "blah"
# Naming after init names everything
derived2 = Derived()
derived2.set_name("derived")
assert derived2.signal1.name == "derived-signal1"
assert derived2.signal2.name == "derived-signal2"
This is inconsistent, so we should fix it.
Possible solutions:
- Remove
name
fromDevice.__init__
and requireDevice.set_name
to be called after creation - Set the name of child devices in
Device.__setattr__
Both of those solutions will break one particular @rtuck99 had (that I can't remember details of) where set_name
was never used after init, so the name "blah" in the above example was actually the desired name.
To support that use case we would have to override Derived.set_name
to explicitly name signal2
to be "blah", either by overriding the baseclass set_name
(for 1):
class Derived(Base):
def set_name(self, name: str, *, child_name_separator: str | None = None) -> None:
super().set_name(name, child_name_separator=child_name_separator)
self.signal2.set_name("blah")
or by making an intermediate function that could be overridden instead (for 2):
class Derived(Base):
def set_child_name(self, child, name):
super().set_child_name(child, "blah" if child is self.signal2 else name)
I think 2 would cause less breakages, so I would favour that over 1, but I'd welcome thoughts on the best API to override the child name.
@DominicOram @DiamondJoseph @callumforrester @jwlodek thoughts?
Acceptance Criteria
- Code changed
- Only the single test that mimics @rtuck99's use case is changed