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 <pedersen+jenkins@frm2.tum.de>
Reviewed-by: Enrico Faulhaber <enrico.faulhaber@frm2.tum.de>
Reviewed-by: Markus Zolliker <markus.zolliker@psi.ch>

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:
+object_counter = itertools.count(1)
-    ctr = [0]
-    def __init__(self):
-        cl = self.__class__.ctr
-        cl[0] += 1
-        self.ctr = cl[0]
 
 
-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(
+        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())]))
+            ['%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
+        if reorder:  # result from apply must use new ctr from Override
-        # self.kwds['ctr'] = self.ctr
+            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)
+        super(Command, self).__init__(ctr, **kwds)
-        if ctr is not None:
-            self.ctr = ctr
 
     @property
     def argument(self):

test/test_modules.py

@@ -128,8 +128,8 @@ def test_ModuleMeta():
         def read_value(self):
             return 0
 
-    sortcheck2 = ['value', 'status', 'target', 'pollinterval',
+    sortcheck2 = ['status', 'target', 'pollinterval',
-                 'param1', 'param2', 'cmd', 'a2', 'cmd2', 'a1', 'b2']
+                  '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)

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(p)
-    q2 = o2.apply(p2)
+    assert q2.ctr == p.ctr   # reorder=False: take ctr from inherited param
-    assert q2.ctr != o2.ctr
+    assert q2 != p
-    assert q2.ctr != p2.ctr  # EVERY override makes a new parameter object -> ctr++
+    assert repr(q2) != repr(p)
-    assert o2.ctr != p2.ctr
+
-    assert q2 != p2
+    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)))