From 04032079d733ab0b0f50c6a43fbac69d232d4466 Mon Sep 17 00:00:00 2001 From: Markus Zolliker Date: Wed, 25 Sep 2019 08:45:42 +0200 Subject: [PATCH] fix some remaining py3 incompatibilites - None <= 0 is invalid in py3 - restrict BLOBType to bytes (may be changed after migration to py3) - remove long - use list(.items()) when dict is changed within loop - allow initialization of properties in HasProperties without supercall to base class - .values() can not be indexed - adapted/removed various tests. additional tests might be added after definitive migration to py3 after this change, all the tests run with py3, also secop-server and secop-gui were tested with an example, but other code might still contain py3 incompatibilities Change-Id: I881c6972aeabb8494a21a6cbc7ffeddfd4f5d4f8 Reviewed-on: https://forge.frm2.tum.de/review/c/sine2020/secop/playground/+/21306 Tested-by: Enrico Faulhaber Reviewed-by: Enrico Faulhaber Reviewed-by: Markus Zolliker Reviewed-by: Bjoern Pedersen --- secop/datatypes.py | 12 ++++----- secop/lib/enum.py | 4 +-- secop/modules.py | 2 +- secop/properties.py | 7 ++--- secop/protocol/interface/tcp.py | 2 +- secop/server.py | 2 +- test/test_datatypes.py | 45 ++++++++++++++++----------------- test/test_properties.py | 13 +++++----- 8 files changed, 42 insertions(+), 45 deletions(-) diff --git a/secop/datatypes.py b/secop/datatypes.py index 857f924..fb3afcc 100644 --- a/secop/datatypes.py +++ b/secop/datatypes.py @@ -127,7 +127,6 @@ class FloatRange(DataType): def __init__(self, minval=None, maxval=None, unit=None, fmtstr=None, absolute_resolution=None, relative_resolution=None,): - self.default = 0 if minval <= 0 <= maxval else minval self._defaults = {} self.set_prop('min', minval, float(u'-inf'), float) self.set_prop('max', maxval, float(u'+inf'), float) @@ -135,6 +134,7 @@ class FloatRange(DataType): self.set_prop('fmtstr', fmtstr, u'%g', unicode) self.set_prop('absolute_resolution', absolute_resolution, 0.0, float) self.set_prop('relative_resolution', relative_resolution, 1.2e-7, float) + self.default = 0 if self.min <= 0 <= self.max else self.min # check values if self.min > self.max: @@ -194,7 +194,7 @@ class IntRange(DataType): def __init__(self, minval=None, maxval=None): self.min = DEFAULT_MIN_INT if minval is None else int(minval) self.max = DEFAULT_MAX_INT if maxval is None else int(maxval) - self.default = 0 if minval <= 0 <= maxval else minval + self.default = 0 if self.min <= 0 <= self.max else self.min # a unit on an int is now allowed in SECoP, but do we need them in Frappy? # self.set_prop('unit', unit, u'', unicode) @@ -245,7 +245,6 @@ class ScaledInteger(DataType): def __init__(self, scale, minval=None, maxval=None, unit=None, fmtstr=None, absolute_resolution=None, relative_resolution=None,): - self.default = 0 if minval <= 0 <= maxval else minval self._defaults = {} self.scale = float(scale) if not self.scale > 0: @@ -257,6 +256,7 @@ class ScaledInteger(DataType): self.min = DEFAULT_MIN_INT * self.scale if minval is None else float(minval) self.max = DEFAULT_MAX_INT * self.scale if maxval is None else float(maxval) + self.default = 0 if self.min <= 0 <= self.max else self.min # check values if self.min > self.max: @@ -386,7 +386,7 @@ class BLOBType(DataType): def __call__(self, value): """return the validated (internal) value or raise""" - if type(value) not in [unicode, str]: + if not isinstance(value, bytes): raise BadValueError(u'%r has the wrong type!' % value) size = len(value) if size < self.minbytes: @@ -399,7 +399,7 @@ class BLOBType(DataType): def export_value(self, value): """returns a python object fit for serialisation""" - return b64encode(value) + return b64encode(value).decode('ascii') def import_value(self, value): """returns a python object from serialisation""" @@ -407,7 +407,7 @@ class BLOBType(DataType): def from_string(self, text): value = text - # XXX: + # XXX: what should we do here? return self(value) def format_value(self, value, unit=None): diff --git a/secop/lib/enum.py b/secop/lib/enum.py index 1a74245..ffc011e 100755 --- a/secop/lib/enum.py +++ b/secop/lib/enum.py @@ -70,7 +70,7 @@ class EnumMember(object): def __eq__(self, other): if isinstance(other, (EnumMember)): return other.value == self.value - if isinstance(other, (int, long)): + if isinstance(other, int): return other == self.value # compare by name (for (in)equality only) if isinstance(other, (str, unicode)): @@ -178,8 +178,6 @@ class EnumMember(object): return self.value.__invert__() def __int__(self): return self.value.__int__() - def __long__(self): - return self.value.__long__() def __float__(self): return self.value.__float__() #return NotImplemented # makes no sense diff --git a/secop/modules.py b/secop/modules.py index fd41b2d..821bada 100644 --- a/secop/modules.py +++ b/secop/modules.py @@ -192,7 +192,7 @@ class Module(HasProperties): # 5) 'apply' config: # pass values through the datatypes and store as attributes - for k, v in cfgdict.items(): + for k, v in list(cfgdict.items()): # apply datatype, complain if type does not fit datatype = self.parameters[k].datatype try: diff --git a/secop/properties.py b/secop/properties.py index 4b76b59..101476f 100644 --- a/secop/properties.py +++ b/secop/properties.py @@ -48,7 +48,7 @@ class Property(object): self.description = description self.default = datatype.default if default is None else datatype(default) self.datatype = datatype - self.extname = unicode(extname) + self.extname = extname self.export = export or bool(extname) self.mandatory = mandatory or (default is None and not isinstance(datatype, ValueType)) self.settable = settable or mandatory # settable means settable from the cfg file @@ -121,8 +121,9 @@ class PropertyMeta(type): class HasProperties(object): properties = {} - def __init__(self, *args): - super(HasProperties, self).__init__() + def __init__(self, supercall_init=True): + if supercall_init: + super(HasProperties, self).__init__() # store property values in the instance, keep descriptors on the class self.properties = {} # pre-init with properties default value (if any) diff --git a/secop/protocol/interface/tcp.py b/secop/protocol/interface/tcp.py index a32d684..8857a9d 100644 --- a/secop/protocol/interface/tcp.py +++ b/secop/protocol/interface/tcp.py @@ -211,7 +211,7 @@ class TCPServer(HasProperties, socketserver.ThreadingTCPServer): self.dispatcher = srv.dispatcher self.name = name self.log = logger - super(TCPServer, self).__init__() + HasProperties.__init__(self, supercall_init=False) bindto = options.pop('bindto', 'localhost') bindport = int(options.pop('bindport', DEF_PORT)) detailed_errors = options.pop('detailed_errors', False) diff --git a/secop/server.py b/secop/server.py index 2a4685a..fef3292 100644 --- a/secop/server.py +++ b/secop/server.py @@ -179,7 +179,7 @@ class Server(object): # following line is the reason for 'node' beeing the first entry in CFGSECTIONS if len(self.nodes) != 1: raise ConfigError(u'cfgfile %r: needs exactly one node section!' % self._cfgfile) - self.dispatcher = self.nodes.values()[0] + self.dispatcher, = tuple(self.nodes.values()) pollTable = dict() # all objs created, now start them up and interconnect diff --git a/test/test_datatypes.py b/test/test_datatypes.py index 6c72231..2c111bf 100644 --- a/test/test_datatypes.py +++ b/test/test_datatypes.py @@ -213,20 +213,21 @@ def test_BLOBType(): with pytest.raises(ValueError): dt(9) with pytest.raises(ValueError): - dt(u'av') + dt(b'av') with pytest.raises(ValueError): - dt(u'abcdefghijklmno') - assert dt('abcd') == b'abcd' + dt(b'abcdefghijklmno') + with pytest.raises(ValueError): + dt(u'abcd') assert dt(b'abcd') == b'abcd' - assert dt(u'abcd') == b'abcd' - assert dt.export_value('abcd') == u'YWJjZA==' assert dt.export_value(b'abcd') == u'YWJjZA==' - assert dt.export_value(u'abcd') == u'YWJjZA==' + assert dt.export_value(b'abcd') == u'YWJjZA==' + # assert dt.export_value(u'abcd') == u'YWJjZA==' assert dt.import_value(u'YWJjZA==') == b'abcd' # XXX: right? or different format? - assert dt.format_value(b'ab\0cd') == '\'ab\\x00cd\'' + # to be added after migration to py3 + # assert dt.format_value(b'ab\0cd') == "b'ab\\x00cd\'" def test_StringType(): @@ -250,16 +251,15 @@ def test_StringType(): dt(u'abcdefghijklmno') with pytest.raises(ValueError): dt('abcdefg\0') - assert dt('abcd') == b'abcd' - assert dt(b'abcd') == b'abcd' - assert dt(u'abcd') == b'abcd' + assert dt('abcd') == 'abcd' + # tests with bytes have to be added after migration to py3 + #assert dt(b'abcd') == 'abcd' - assert dt.export_value('abcd') == b'abcd' - assert dt.export_value(b'abcd') == b'abcd' - assert dt.export_value(u'abcd') == b'abcd' - assert dt.import_value(u'abcd') == u'abcd' + assert dt.export_value('abcd') == 'abcd' + # assert dt.export_value(b'abcd') == 'abcd' + assert dt.import_value('abcd') == 'abcd' - assert dt.format_value(u'abcd') == u"u'abcd'" + assert dt.format_value('abcd') == "'abcd'" def test_TextType(): @@ -274,14 +274,13 @@ def test_TextType(): dt(u'abcdefghijklmno') with pytest.raises(ValueError): dt('abcdefg\0') - assert dt('ab\n\ncd\n') == b'ab\n\ncd\n' - assert dt(b'ab\n\ncd\n') == b'ab\n\ncd\n' - assert dt(u'ab\n\ncd\n') == b'ab\n\ncd\n' + assert dt('ab\n\ncd\n') == 'ab\n\ncd\n' + # assert dt(b'ab\n\ncd\n') == 'ab\n\ncd\n' - assert dt.export_value('abcd') == b'abcd' - assert dt.export_value(b'abcd') == b'abcd' - assert dt.export_value(u'abcd') == b'abcd' - assert dt.import_value(u'abcd') == u'abcd' + assert dt.export_value('abcd') == 'abcd' + # assert dt.export_value(b'abcd') == b'abcd' + assert dt.export_value('abcd') == 'abcd' + assert dt.import_value('abcd') == 'abcd' def test_BoolType(): @@ -395,7 +394,7 @@ def test_StructOf(): assert dt.import_value({u'an_int': 13, u'a_string': u'WFEC'}) == { u'a_string': u'WFEC', u'an_int': 13} - assert dt.format_value({u'an_int':2, u'a_string':u'Z'}) == u"{a_string=u'Z', an_int=2}" + assert dt.format_value({'an_int':2, u'a_string':'Z'}) == u"{a_string='Z', an_int=2}" def test_Command(): diff --git a/test/test_properties.py b/test/test_properties.py index bfea463..6ef0d9c 100644 --- a/test/test_properties.py +++ b/test/test_properties.py @@ -47,16 +47,15 @@ V_test_Property = [ [(IntRange(), 0, '', False, False), dict(default=0, extname='', export=False, mandatory=False)], [(IntRange(), None, '', False, False), - dict(default=None, extname='', export=False, mandatory=True)], # 'normal types + no default -> mandatory + dict(default=0, extname='', export=False, mandatory=True)], # 'normal types + no default -> mandatory [(ValueType(), None, '', False, False), dict(default=None, extname='', export=False, mandatory=False)], # 'special type + no default -> NOT mandatory ] -def test_Property(): - for entry in V_test_Property: - args, check = entry - p = Property('', *args) - for k,v in check.items(): - assert getattr(p, k) == v +@pytest.mark.parametrize('args, check', V_test_Property) +def test_Property(args, check): + p = Property('', *args) + for k,v in check.items(): + assert getattr(p, k) == v def test_Property_basic(): with pytest.raises(TypeError):