At the moment ControlSystem looks like this:
def __init__(self):
ElementHolder.__init__(self)
@abstractmethod
def name(self) -> str:
"""Return control system name (i.e. live)"""
pass
This interface means two things:
-
There is no automatic handling of the name attribute. That has to be implemented in every subclass which means code repetition. Before when we had the ConfigModels that might have been required but now I think we can define already here that a ControlSystem should be initialized with a name to define that as the minimum required interface for the constructor.
-
There is no property for the name. It has to be called as self.name() rather than self.name. Since name is an attribute I think it should be turned into a property to make the interface easier for the users to understand.
My suggestion is to change it to:
def __init__(self, name: str):
self._name = name
ElementHolder.__init__(self)
@property
def name(self) -> str:
return self._name
We can then also later change to use the __pyaml__repr__ for the string representation and the name will automatically be included.
The change will however cause incompatibility issues with tango-pyaml and pyaml-cs-oa that need to be handled so that's why I didn't want to make a PR before first suggesting the idea.
At the moment ControlSystem looks like this:
This interface means two things:
There is no automatic handling of the name attribute. That has to be implemented in every subclass which means code repetition. Before when we had the ConfigModels that might have been required but now I think we can define already here that a ControlSystem should be initialized with a name to define that as the minimum required interface for the constructor.
There is no property for the name. It has to be called as
self.name()rather thanself.name. Since name is an attribute I think it should be turned into a property to make the interface easier for the users to understand.My suggestion is to change it to:
We can then also later change to use the
__pyaml__repr__for the string representation and the name will automatically be included.The change will however cause incompatibility issues with tango-pyaml and pyaml-cs-oa that need to be handled so that's why I didn't want to make a PR before first suggesting the idea.