fix parameter inheritance

Correct inheritance has to follow the MRO, not only consider
the direct base classes.

Patchset 3: changed only tests, indicating that we need to change the code

Following patchsets include a major change in params.py and
modules.py. The parameter properties for inheritance, corresponding
mainly to the constructor arguments have to be stored separately
from the property values including inherited stuff.

Change-Id: Ibcbccb6abcc22e7e2d91df8f70ef64226684d8cc
Reviewed-on: https://forge.frm2.tum.de/review/c/sine2020/secop/playground/+/26805
Reviewed-by: Enrico Faulhaber <enrico.faulhaber@frm2.tum.de>
Reviewed-by: Markus Zolliker <markus.zolliker@psi.ch>
Tested-by: Jenkins Automated Tests <pedersen+jenkins@frm2.tum.de>
This commit is contained in:
zolliker 2021-10-05 16:04:50 +02:00
parent 41489a4a24
commit 3b7cc33f64
6 changed files with 350 additions and 138 deletions

View File

@ -958,7 +958,7 @@ class CommandType(DataType):
argstr = repr(self.argument) if self.argument else ''
if self.result is None:
return 'CommandType(%s)' % argstr
return 'CommandType(%s)->%s' % (argstr, repr(self.result))
return 'CommandType(%s, %s)' % (argstr, repr(self.result))
def __call__(self, value):
"""return the validated argument value or raise"""
@ -999,7 +999,10 @@ class DataTypeType(DataType):
returns the value or raises an appropriate exception"""
if isinstance(value, DataType):
return value
raise ProgrammingError('%r should be a DataType!' % value)
try:
return get_datatype(value)
except Exception as e:
raise ProgrammingError(e) from None
def export_value(self, value):
"""if needed, reformat value for transport"""
@ -1034,6 +1037,13 @@ class ValueType(DataType):
"""
raise NotImplementedError
def setProperty(self, key, value):
"""silently ignored
as ValueType is used for the datatype default, this makes code
shorter for cases, where the datatype may not yet be defined
"""
class NoneOr(DataType):
"""validates a None or smth. else"""

View File

@ -25,9 +25,10 @@
import sys
import time
from collections import OrderedDict
from secop.datatypes import ArrayOf, BoolType, EnumType, FloatRange, \
IntRange, StatusType, StringType, TextType, TupleOf, get_datatype
IntRange, StatusType, StringType, TextType, TupleOf
from secop.errors import BadValueError, ConfigError, InternalError, \
ProgrammingError, SECoPError, SilentError, secop_error
from secop.lib import formatException, mkthread
@ -40,7 +41,7 @@ Done = object() #: a special return value for a read/write function indicating
class HasAccessibles(HasProperties):
"""base class of module
"""base class of Module
joining the class's properties, parameters and commands dicts with
those of base classes.
@ -52,40 +53,46 @@ class HasAccessibles(HasProperties):
super().__init_subclass__()
# merge accessibles from all sub-classes, treat overrides
# for now, allow to use also the old syntax (parameters/commands dict)
accessibles = {}
for base in reversed(cls.__bases__):
accessibles.update(getattr(base, 'accessibles', {}))
newaccessibles = {k: v for k, v in cls.__dict__.items() if isinstance(v, Accessible)}
for aname, aobj in list(accessibles.items()):
value = getattr(cls, aname, None)
if not isinstance(value, Accessible): # else override is already done in __set_name__
if value is None:
accessibles.pop(aname)
else:
# this is either a method overwriting a command
# or a value overwriting a property value or parameter default
anew = aobj.override(value)
newaccessibles[aname] = anew
setattr(cls, aname, anew)
anew.__set_name__(cls, aname)
accessibles = OrderedDict() # dict of accessibles
merged_properties = {} # dict of dict of merged properties
new_names = [] # list of names of new accessibles
override_values = {} # bare values overriding a parameter and methods overriding a command
ordered = {}
for aname in cls.__dict__.get('paramOrder', ()):
for base in reversed(cls.__mro__):
for key, value in base.__dict__.items():
if isinstance(value, Accessible):
value.updateProperties(merged_properties.setdefault(key, {}))
if base == cls and key not in accessibles:
new_names.append(key)
accessibles[key] = value
override_values.pop(key, None)
elif key in accessibles:
override_values[key] = value
for aname, aobj in accessibles.items():
if aname in override_values:
aobj = aobj.copy()
aobj.merge(merged_properties[aname])
aobj.override(override_values[aname])
# replace the bare value by the created accessible
setattr(cls, aname, aobj)
else:
aobj.merge(merged_properties[aname])
accessibles[aname] = aobj
# rebuild order: (1) inherited items, (2) items from paramOrder, (3) new accessibles
# move (2) to the end
for aname in list(cls.__dict__.get('paramOrder', ())):
if aname in accessibles:
ordered[aname] = accessibles.pop(aname)
elif aname in newaccessibles:
ordered[aname] = newaccessibles.pop(aname)
accessibles.move_to_end(aname)
# ignore unknown names
# starting from old accessibles not mentioned, append items from 'order'
accessibles.update(ordered)
# then new accessibles not mentioned
accessibles.update(newaccessibles)
# move (3) to the end
for aname in new_names:
accessibles.move_to_end(aname)
# note: for python < 3.6 the order of inherited items is not ensured between
# declarations within the same class
cls.accessibles = accessibles
# Correct naming of EnumTypes
for k, v in accessibles.items():
if isinstance(v, Parameter) and isinstance(v.datatype, EnumType):
v.datatype.set_name(k)
# moved to Parameter.__set_name__
# check validity of Parameter entries
for pname, pobj in accessibles.items():
@ -318,8 +325,9 @@ class Module(HasAccessibles):
paramobj = self.accessibles.get(paramname, None)
# paramobj might also be a command (not sure if this is needed)
if paramobj:
if propname == 'datatype':
propvalue = get_datatype(propvalue, k)
# no longer needed, this conversion is done by DataTypeType.__call__:
# if propname == 'datatype':
# propvalue = get_datatype(propvalue, k)
try:
paramobj.setProperty(propname, propvalue)
except KeyError:
@ -347,6 +355,10 @@ class Module(HasAccessibles):
self.valueCallbacks[pname] = []
self.errorCallbacks[pname] = []
if not pobj.hasDatatype():
errors.append('%s needs a datatype' % pname)
continue
if pname in cfgdict:
if not pobj.readonly and pobj.initwrite is not False:
# parameters given in cfgdict have to call write_<pname>
@ -393,11 +405,15 @@ class Module(HasAccessibles):
cfgdict.pop(k)
except (ValueError, TypeError) as e:
# self.log.exception(formatExtendedStack())
errors.append('module %s, parameter %s: %s' % (self.name, k, e))
errors.append('parameter %s: %s' % (k, e))
# ensure consistency
for aobj in self.accessibles.values():
aobj.finish()
# Modify units AFTER applying the cfgdict
for k, v in self.parameters.items():
dt = v.datatype
for pname, pobj in self.parameters.items():
dt = pobj.datatype
if '$' in dt.unit:
dt.setProperty('unit', dt.unit.replace('$', self.parameters['value'].datatype.unit))

View File

@ -35,9 +35,17 @@ UNSET = object() # an argument not given, not even None
class Accessible(HasProperties):
"""base class for Parameter and Command"""
"""base class for Parameter and Command
kwds = None # is a dict if it might be used as Override
Inheritance mechanism:
param.propertyValues contains the properties, which will be used when the
owner class will be instantiated
param.ownProperties contains the properties to be used for inheritance
"""
ownProperties = None
def init(self, kwds):
# do not use self.propertyValues.update here, as no invalid values should be
@ -45,42 +53,43 @@ class Accessible(HasProperties):
for k, v in kwds.items():
self.setProperty(k, v)
def inherit(self, cls, owner):
for base in owner.__bases__:
if hasattr(base, self.name):
aobj = getattr(base, 'accessibles', {}).get(self.name)
if aobj:
if not isinstance(aobj, cls):
raise ProgrammingError('%s %s.%s can not inherit from a %s' %
(cls.__name__, owner.__name__, self.name, aobj.__class__.__name__))
# inherit from aobj
for pname, value in aobj.propertyValues.items():
if pname not in self.propertyValues:
self.propertyValues[pname] = value
break
def as_dict(self):
return self.propertyValues
def override(self, value=UNSET, **kwds):
"""return a copy, overridden by a bare attribute
and/or some properties"""
def override(self, value):
"""override with a bare value"""
raise NotImplementedError
def copy(self):
"""return a (deep) copy of ourselfs"""
raise NotImplementedError
def updateProperties(self, merged_properties):
"""update merged_properties with our own properties"""
raise NotImplementedError
def merge(self, merged_properties):
"""merge with inherited properties
:param merged_properties: dict of properties to be updated
note: merged_properties may be modified
"""
raise NotImplementedError
def finish(self):
"""ensure consistency"""
raise NotImplementedError
def for_export(self):
"""prepare for serialisation"""
raise NotImplementedError
def hasDatatype(self):
return 'datatype' in self.propertyValues
def __repr__(self):
props = []
for k, prop in sorted(self.propertyDict.items()):
v = self.propertyValues.get(k, prop.default)
if v != prop.default:
for k, v in sorted(self.propertyValues.items()):
props.append('%s=%r' % (k, v))
return '%s(%s)' % (self.__class__.__name__, ', '.join(props))
@ -100,7 +109,7 @@ class Parameter(Accessible):
extname='description', mandatory=True, export='always')
datatype = Property(
'datatype of the Parameter (SECoP datainfo)', DataTypeType(),
extname='datainfo', mandatory=True, export='always')
extname='datainfo', mandatory=True, export='always', default=ValueType())
readonly = Property(
'not changeable via SECoP (default True)', BoolType(),
extname='readonly', default=True, export='always')
@ -160,9 +169,13 @@ class Parameter(Accessible):
timestamp = 0
readerror = None
def __init__(self, description=None, datatype=None, inherit=True, *, unit=None, constant=None, **kwds):
def __init__(self, description=None, datatype=None, inherit=True, **kwds):
super().__init__()
if datatype is not None:
if datatype is None:
# collect datatype properties. these are not applied, as we have no datatype
self.ownProperties = {k: kwds.pop(k) for k in list(kwds) if k not in self.propertyDict}
else:
self.ownProperties = {}
if not isinstance(datatype, DataType):
if isinstance(datatype, type) and issubclass(datatype, DataType):
# goodie: make an instance from a class (forgotten ()???)
@ -174,15 +187,15 @@ class Parameter(Accessible):
if 'default' in kwds:
self.default = datatype(kwds['default'])
self.init(kwds) # datatype must be defined before we can treat dataset properties like fmtstr or unit
if description is not None:
self.description = inspect.cleandoc(description)
kwds['description'] = inspect.cleandoc(description)
# save for __set_name__
self._inherit = inherit
self._unit = unit # for legacy code only
self._constant = constant
self.init(kwds)
if inherit:
self.ownProperties.update(self.propertyValues)
else:
self.ownProperties = {k: getattr(self, k) for k in self.propertyDict}
def __get__(self, instance, owner):
# not used yet
@ -195,57 +208,70 @@ class Parameter(Accessible):
def __set_name__(self, owner, name):
self.name = name
if isinstance(self.datatype, EnumType):
self.datatype.set_name(name)
if self._inherit:
self.inherit(Parameter, owner)
if self.export is True:
predefined_cls = PREDEFINED_ACCESSIBLES.get(self.name, None)
if predefined_cls is Parameter:
self.export = self.name
elif predefined_cls is None:
self.export = '_' + self.name
else:
raise ProgrammingError('can not use %r as name of a Parameter' % self.name)
# check for completeness
missing_properties = [pname for pname in ('description', 'datatype') if pname not in self.propertyValues]
if missing_properties:
raise ProgrammingError('Parameter %s.%s needs a %s' %
(owner.__name__, name, ' and a '.join(missing_properties)))
if self._unit is not None:
self.datatype.setProperty('unit', self._unit)
def copy(self):
"""return a (deep) copy of ourselfs"""
res = type(self)()
res.name = self.name
res.init(self.propertyValues)
if 'datatype' in self.propertyValues:
res.datatype = res.datatype.copy()
return res
if self._constant is not None:
constant = self.datatype(self._constant)
def updateProperties(self, merged_properties):
"""update merged_properties with our own properties"""
datatype = self.ownProperties.get('datatype')
if datatype is not None:
# clear datatype properties, as they are overriden by datatype
for key in list(merged_properties):
if key not in self.propertyDict:
merged_properties.pop(key)
merged_properties.update(self.ownProperties)
def override(self, value):
"""override default"""
self.default = self.datatype(value)
def merge(self, merged_properties):
"""merge with inherited properties
:param merged_properties: dict of properties to be updated
note: merged_properties may be modified
"""
datatype = merged_properties.pop('datatype', None)
if datatype is not None:
self.datatype = datatype.copy()
self.init(merged_properties)
self.finish()
def finish(self):
"""ensure consistency"""
if self.constant is not None:
constant = self.datatype(self.constant)
# The value of the `constant` property should be the
# serialised version of the constant, or unset
self.constant = self.datatype.export_value(constant)
self.readonly = True
if 'default' in self.propertyValues:
# fixes in case datatype has changed
try:
self.datatype(self.default)
self.default = self.datatype(self.default)
except BadValueError:
# clear default, if it does not match datatype
self.propertyValues.pop('default')
if self.export is True:
predefined_cls = PREDEFINED_ACCESSIBLES.get(name, None)
if predefined_cls is Parameter:
self.export = name
elif predefined_cls is None:
self.export = '_' + name
else:
raise ProgrammingError('can not use %r as name of a Parameter' % name)
def copy(self):
# deep copy, as datatype might be altered from config
res = type(self)()
res.name = self.name
res.init(self.propertyValues)
res.datatype = res.datatype.copy()
return res
def override(self, value=UNSET, **kwds):
res = self.copy()
res.init(kwds)
if value is not UNSET:
res.value = res.datatype(value)
return res
def export_value(self):
return self.datatype.export_value(self.value)
@ -255,15 +281,23 @@ class Parameter(Accessible):
def getProperties(self):
"""get also properties of datatype"""
super_prop = super().getProperties().copy()
if self.datatype:
super_prop.update(self.datatype.getProperties())
return super_prop
def setProperty(self, key, value):
"""set also properties of datatype"""
try:
if key in self.propertyDict:
super().setProperty(key, value)
else:
try:
self.datatype.setProperty(key, value)
except KeyError:
raise ProgrammingError('cannot set %s on parameter with datatype %s'
% (key, type(self.datatype).__name__)) from None
except ValueError as e:
raise ProgrammingError('property %s: %s' % (key, str(e))) from None
def checkProperties(self):
super().checkProperties()
@ -336,10 +370,9 @@ class Command(Accessible):
if self.func is None:
raise ProgrammingError('Command %s.%s must be used as a method decorator' %
(owner.__name__, name))
if self._inherit:
self.inherit(Command, owner)
self.datatype = CommandType(self.argument, self.result)
self.ownProperties = self.propertyValues.copy()
if self.export is True:
predefined_cls = PREDEFINED_ACCESSIBLES.get(name, None)
if predefined_cls is Command:
@ -347,22 +380,28 @@ class Command(Accessible):
elif predefined_cls is None:
self.export = '_' + name
else:
raise ProgrammingError('can not use %r as name of a Command' % name)
raise ProgrammingError('can not use %r as name of a Command' % name) from None
if not self._inherit:
for key, pobj in self.properties.items():
if key not in self.propertyValues:
self.propertyValues[key] = pobj.default
def __get__(self, obj, owner=None):
if obj is None:
return self
if not self.func:
raise ProgrammingError('Command %s not properly configured' % self.name)
raise ProgrammingError('Command %s not properly configured' % self.name) from None
return self.func.__get__(obj, owner)
def __call__(self, func):
"""called when used as decorator"""
if 'description' not in self.propertyValues and func.__doc__:
self.description = inspect.cleandoc(func.__doc__)
self.func = func
return self
def copy(self):
"""return a (deep) copy of ourselfs"""
res = type(self)()
res.name = self.name
res.func = self.func
@ -371,15 +410,45 @@ class Command(Accessible):
res.argument = res.argument.copy()
if res.result:
res.result = res.result.copy()
res.datatype = CommandType(res.argument, res.result)
self.finish()
return res
def override(self, value=UNSET, **kwds):
res = self.copy()
res.init(kwds)
if value is not UNSET:
res.func = value
return res
def updateProperties(self, merged_properties):
"""update merged_properties with our own properties"""
merged_properties.update(self.ownProperties)
def override(self, value):
"""override method
this is needed when the @Command is missing on a method overriding a command"""
if not callable(value):
raise ProgrammingError('%s = %r is overriding a Command' % (self.name, value))
self.func = value
if value.__doc__:
self.description = inspect.cleandoc(value.__doc__)
def merge(self, merged_properties):
"""merge with inherited properties
:param merged_properties: dict of properties to be updated
"""
self.init(merged_properties)
self.finish()
def finish(self):
"""ensure consistency"""
self.datatype = CommandType(self.argument, self.result)
def setProperty(self, key, value):
"""special treatment of datatype"""
try:
if key == 'datatype':
command = DataTypeType()(value)
super().setProperty('argument', command.argument)
super().setProperty('result', command.result)
super().setProperty(key, value)
except ValueError as e:
raise ProgrammingError('property %s: %s' % (key, str(e))) from None
def do(self, module_obj, argument):
"""perform function call

View File

@ -182,7 +182,7 @@ def proxy_class(remote_class, name=None):
for aname, aobj in rcls.accessibles.items():
if isinstance(aobj, Parameter):
pobj = aobj.override(poll=False, handler=None, needscfg=False)
pobj = aobj.merge(dict(poll=False, handler=None, needscfg=False))
attrs[aname] = pobj
def rfunc(self, pname=aname):

View File

@ -26,14 +26,16 @@ import threading
import pytest
from secop.datatypes import BoolType, FloatRange, StringType
from secop.errors import ProgrammingError
from secop.modules import Communicator, Drivable, Module
from secop.datatypes import BoolType, FloatRange, StringType, IntRange
from secop.errors import ProgrammingError, ConfigError
from secop.modules import Communicator, Drivable, Readable, Module
from secop.params import Command, Parameter
from secop.poller import BasicPoller
class DispatcherStub:
OMIT_UNCHANGED_WITHIN = 0
def __init__(self, updates):
self.updates = updates
@ -51,6 +53,9 @@ class LoggerStub:
info = warning = exception = debug
logger = LoggerStub()
class ServerStub:
def __init__(self, updates):
self.dispatcher = DispatcherStub(updates)
@ -142,7 +147,6 @@ def test_ModuleMagic():
sortcheck2 = ['status', 'pollinterval', 'target', 'stop',
'a1', 'a2', 'cmd2', 'param1', 'param2', 'cmd', 'value', 'b2']
logger = LoggerStub()
updates = {}
srv = ServerStub(updates)
@ -228,3 +232,113 @@ def test_ModuleMagic():
o.earlyInit()
for o in objects:
o.initModule()
def test_param_inheritance():
srv = ServerStub({})
class Base(Module):
param = Parameter()
class MissingDatatype(Base):
param = Parameter('param')
class MissingDescription(Base):
param = Parameter(datatype=FloatRange(), default=0)
# missing datatype and/or description of a parameter has to be detected
# at instantation and only then
with pytest.raises(ConfigError) as e_info:
MissingDatatype('o', logger, {'description': ''}, srv)
assert 'datatype' in repr(e_info.value)
with pytest.raises(ConfigError) as e_info:
MissingDescription('o', logger, {'description': ''}, srv)
assert 'description' in repr(e_info.value)
with pytest.raises(ConfigError) as e_info:
Base('o', logger, {'description': ''}, srv)
def test_mixin():
# srv = ServerStub({})
class Mixin: # no need to inherit from Module or HasAccessible
value = Parameter(unit='K') # missing datatype and description acceptable in mixins
param1 = Parameter('no datatype yet', fmtstr='%.5f')
param2 = Parameter('no datatype yet', default=1)
class MixedReadable(Mixin, Readable):
pass
class MixedDrivable(MixedReadable, Drivable):
value = Parameter(unit='Ohm', fmtstr='%.3f')
param1 = Parameter(datatype=FloatRange())
with pytest.raises(ProgrammingError):
class MixedModule(Mixin): # pylint: disable=unused-variable
param1 = Parameter('', FloatRange(), fmtstr=0) # fmtstr must be a string
assert repr(MixedDrivable.status.datatype) == repr(Drivable.status.datatype)
assert repr(MixedReadable.status.datatype) == repr(Readable.status.datatype)
assert MixedReadable.value.datatype.unit == 'K'
assert MixedDrivable.value.datatype.unit == 'Ohm'
assert MixedDrivable.value.datatype.fmtstr == '%.3f'
# when datatype is overridden, fmtstr falls back to default:
assert MixedDrivable.param1.datatype.fmtstr == '%g'
srv = ServerStub({})
MixedDrivable('o', logger, {
'description': '',
'param1.description': 'param 1',
'param1': 0,
'param2.datatype': {"type": "double"},
}, srv)
with pytest.raises(ConfigError):
MixedReadable('o', logger, {
'description': '',
'param1.description': 'param 1',
'param1': 0,
'param2.datatype': {"type": "double"},
}, srv)
def test_override():
class Mod(Drivable):
value = 5 # overriding the default value
def stop(self):
"""no decorator needed"""
assert Mod.value.default == 5
assert Mod.stop.description == "no decorator needed"
def test_command_config():
class Mod(Module):
@Command(IntRange(0, 1), result=IntRange(0, 1))
def convert(self, value):
return value
srv = ServerStub({})
mod = Mod('o', logger, {
'description': '',
'convert.argument': {'type': 'bool'},
}, srv)
assert mod.commands['convert'].datatype.export_datatype() == {
'type': 'command',
'argument': {'type': 'bool'},
'result': {'type': 'int', 'min': 0, 'max': 1},
}
mod = Mod('o', logger, {
'description': '',
'convert.datatype': {'type': 'command', 'argument': {'type': 'bool'}, 'result': {'type': 'bool'}},
}, srv)
assert mod.commands['convert'].datatype.export_datatype() == {
'type': 'command',
'argument': {'type': 'bool'},
'result': {'type': 'bool'},
}

View File

@ -88,15 +88,18 @@ def test_Override():
p1 = Parameter(default=True)
p2 = Parameter() # override without change
assert Mod.p1 != Base.p1
assert Mod.p2 != Base.p2
assert Mod.p3 == Base.p3
assert id(Mod.p2) != id(Base.p2) # must be a new object
assert repr(Mod.p2) == repr(Base.p2) # but must be a clone
assert id(Mod.p1) != id(Base.p1)
assert id(Mod.p2) != id(Base.p2)
assert id(Mod.p3) == id(Base.p3)
assert repr(Mod.p2) == repr(Base.p2) # must be a clone
assert repr(Mod.p3) == repr(Base.p3) # must be a clone
assert Mod.p1.default is True
# manipulating default makes Base.p1 and Mod.p1 match
Mod.p1.default = False
assert repr(Mod.p1) == repr(Base.p1)
def test_Export():
class Mod:
class Mod(HasAccessibles):
param = Parameter('description1', datatype=BoolType, default=False)
assert Mod.param.export == '_param'