From 9df43bb940b16f12d67a305bc6c33de24432f0b0 Mon Sep 17 00:00:00 2001 From: Markus Zolliker Date: Mon, 25 Jan 2021 18:26:13 +0100 Subject: [PATCH] make order of accessibles work again - when applying overrides with reorder=True, take ctr from Override, else copy from the cloned Accesible. This did not work properly - reworked: - replaced CountedObj class by object_counter - accessibles created by a copy or by applying Overrides do not need fresh counted values - adjusted tests Change-Id: Id2fcf1ab1295aa1ea80ea81ae8cd02d36f86e969 Reviewed-on: https://forge.frm2.tum.de/review/c/sine2020/secop/playground/+/24926 Tested-by: Jenkins Automated Tests Reviewed-by: Enrico Faulhaber Reviewed-by: Markus Zolliker --- secop/params.py | 44 ++++++++++++++++---------------------------- test/test_modules.py | 7 +++---- test/test_params.py | 32 +++++++++++++++++--------------- 3 files changed, 36 insertions(+), 47 deletions(-) diff --git a/secop/params.py b/secop/params.py index af26423..b951c00 100644 --- a/secop/params.py +++ b/secop/params.py @@ -24,6 +24,7 @@ from collections import OrderedDict +import itertools from secop.datatypes import CommandType, DataType, StringType, BoolType, EnumType, DataTypeType, ValueType, OrType, \ NoneOr, TextType, IntRange @@ -31,20 +32,16 @@ from secop.errors import ProgrammingError, BadValueError from secop.properties import HasProperties, Property -class CountedObj: - ctr = [0] - def __init__(self): - cl = self.__class__.ctr - cl[0] += 1 - self.ctr = cl[0] +object_counter = itertools.count(1) -class Accessible(HasProperties, CountedObj): +class Accessible(HasProperties): '''base class for Parameter and Command''' properties = {} - def __init__(self, **kwds): + def __init__(self, ctr, **kwds): + self.ctr = ctr or next(object_counter) super(Accessible, self).__init__() # do not use self.properties.update here, as no invalid values should be # assigned to properties, even not before checkProperties @@ -52,8 +49,9 @@ class Accessible(HasProperties, CountedObj): self.setProperty(k, v) def __repr__(self): - return '%s_%d(%s)' % (self.__class__.__name__, self.ctr, ',\n\t'.join( - ['%s=%r' % (k, self.properties.get(k, v.default)) for k, v in sorted(self.__class__.properties.items())])) + return '%s(%s, ctr=%d)' % (self.__class__.__name__, ',\n\t'.join( + ['%s=%r' % (k, self.properties.get(k, v.default)) for k, v in sorted(self.__class__.properties.items())]), + self.ctr) def copy(self): # return a copy of ourselfs @@ -122,9 +120,6 @@ class Parameter(Accessible): def __init__(self, description, datatype, ctr=None, unit=None, **kwds): - if ctr is not None: - self.ctr = ctr - if not isinstance(datatype, DataType): if issubclass(datatype, DataType): # goodie: make an instance from a class (forgotten ()???) @@ -138,8 +133,7 @@ class Parameter(Accessible): kwds['readonly'] = kwds.get('readonly', True) # for frappy optional, for SECoP mandatory if unit is not None: # for legacy code only datatype.setProperty('unit', unit) - super(Parameter, self).__init__(**kwds) - + super(Parameter, self).__init__(ctr, **kwds) if self.initwrite and self.readonly: raise ProgrammingError('can not have both readonly and initwrite!') @@ -221,24 +215,22 @@ class Commands(Parameters): """class storage for Commands""" -class Override(CountedObj): +class Override: """Stores the overrides to be applied to a Parameter note: overrides are applied by the metaclass during class creating reorder= True: use position of Override instead of inherited for the order """ def __init__(self, description="", reorder=False, **kwds): - super(Override, self).__init__() self.kwds = kwds - self.reorder = reorder # allow to override description without keyword if description: self.kwds['description'] = description - # for now, do not use the Override ctr - # self.kwds['ctr'] = self.ctr + if reorder: # result from apply must use new ctr from Override + self.kwds['ctr'] = next(object_counter) def __repr__(self): - return '%s_%d(%s)' % (self.__class__.__name__, self.ctr, ', '.join( + return '%s(%s)' % (self.__class__.__name__, ', '.join( ['%s=%r' % (k, v) for k, v in sorted(self.kwds.items())])) def apply(self, obj): @@ -256,12 +248,10 @@ class Override(CountedObj): except BadValueError: # clear default, if it does not match datatype props['default'] = None + props['ctr'] = obj.ctr # take ctr from inherited param except when overridden by self.kwds props.update(self.kwds) - - if self.reorder: - #props['ctr'] = self.ctr - return type(obj)(ctr=self.ctr, **props) return type(obj)(**props) + raise ProgrammingError( "Overrides can only be applied to Accessibles, %r is none!" % obj) @@ -292,9 +282,7 @@ class Command(Accessible): def __init__(self, description, ctr=None, **kwds): kwds['description'] = description kwds['datatype'] = CommandType(kwds.get('argument', None), kwds.get('result', None)) - super(Command, self).__init__(**kwds) - if ctr is not None: - self.ctr = ctr + super(Command, self).__init__(ctr, **kwds) @property def argument(self): diff --git a/test/test_modules.py b/test/test_modules.py index 9d4ae97..9de860a 100644 --- a/test/test_modules.py +++ b/test/test_modules.py @@ -128,8 +128,8 @@ def test_ModuleMeta(): def read_value(self): return 0 - sortcheck2 = ['value', 'status', 'target', 'pollinterval', - 'param1', 'param2', 'cmd', 'a2', 'cmd2', 'a1', 'b2'] + sortcheck2 = ['status', 'target', 'pollinterval', + 'param1', 'param2', 'cmd', 'a2', 'cmd2', 'value', 'a1', 'b2'] logger = LoggerStub() updates = {} @@ -151,8 +151,7 @@ def test_ModuleMeta(): assert o.ctr not in ctr_found ctr_found.add(o.ctr) check_order = [(obj.accessibles[n].ctr, n) for n in sortcheck] - # HACK: atm. disabled to fix all other problems first. - assert check_order + sorted(check_order) + assert check_order == sorted(check_order) # check for inital updates working properly o1 = Newclass1('o1', logger, {'.description':''}, srv) diff --git a/test/test_params.py b/test/test_params.py index ef4c6c8..92702ec 100644 --- a/test/test_params.py +++ b/test/test_params.py @@ -59,7 +59,7 @@ def test_Parameter(): with pytest.raises(ProgrammingError): Parameter(None, datatype=float) p3 = p1.copy() - assert p1.ctr != p3.ctr + assert p1.ctr == p3.ctr p3.ctr = p1.ctr # manipulate ctr for next line assert repr(p1) == repr(p3) assert p1.datatype != p2.datatype @@ -67,22 +67,24 @@ def test_Parameter(): def test_Override(): p = Parameter('description1', datatype=BoolType, default=False) - o = Override(default=True, reorder=True) - assert o.ctr != p.ctr - q = o.apply(p) - assert q.ctr != o.ctr # override shall be useable to influence the order, hence copy the ctr value - assert q.ctr != p.ctr - assert o.ctr != p.ctr - assert q != p - p2 = Parameter('description2', datatype=BoolType, default=False) + o = Override(default=True, reorder=True) + q = o.apply(p) + qctr = q.ctr + assert q.ctr > p.ctr # reorder=True: take ctr from override object + assert q != p + assert qctr == o.apply(p).ctr # do not create a new ctr when applied again + o2 = Override(default=True) - assert o2.ctr != p2.ctr - q2 = o2.apply(p2) - assert q2.ctr != o2.ctr - assert q2.ctr != p2.ctr # EVERY override makes a new parameter object -> ctr++ - assert o2.ctr != p2.ctr - assert q2 != p2 + q2 = o2.apply(p) + assert q2.ctr == p.ctr # reorder=False: take ctr from inherited param + assert q2 != p + assert repr(q2) != repr(p) + + q3 = Override().apply(p) # Override without change + assert id(q2) != id(p) # must be a new object + assert repr(q3) == repr(p) # but must be a clone + def test_Parameters(): ps = Parameters(dict(p1=Parameter('p1', datatype=BoolType, default=True)))