From 8835d164016bcf69a36dd718a120594f425fdf3e Mon Sep 17 00:00:00 2001 From: Steinar Sturlaugsson Date: Fri, 9 Apr 2021 16:11:19 +0200 Subject: [PATCH 1/5] Use a more readable syntax for parsing booleans It's probably more clear to most readers to use this syntax here: value = 'true' if value else 'false' than the old value = value and 'true' or 'false' Given that value has one of three values, {True, False, None}, both statements are equivalent. This can be checked by executing: all([(x and 'true' or 'false') == ('true' if x else 'false') for x in {True, False, None}]) --- genologics/descriptors.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/genologics/descriptors.py b/genologics/descriptors.py index 735b82b..5b0235b 100644 --- a/genologics/descriptors.py +++ b/genologics/descriptors.py @@ -261,7 +261,7 @@ def __setitem__(self, key, value): elif vtype == 'boolean': if not isinstance(value, bool): raise TypeError('Boolean UDF requires bool value') - value = value and 'true' or 'false' + value = 'true' if value else 'false' elif vtype == 'date': if not isinstance(value, datetime.date): # Too restrictive? raise TypeError('Date UDF requires datetime.date value') @@ -282,10 +282,10 @@ def __setitem__(self, key, value): break else: # Create new entry; heuristics for type if self._is_string(value): - vtype = '\n' in value and 'Text' or 'String' + vtype = 'Text' if '\n' in value else 'String' elif isinstance(value, bool): vtype = 'Boolean' - value = value and 'true' or 'false' + value = 'true' if value else 'false' elif isinstance(value, (int, float, Decimal)): vtype = 'Numeric' value = str(value) From d6c49773cef3d01db7c2f82727885423160559d2 Mon Sep 17 00:00:00 2001 From: Steinar Sturlaugsson Date: Fri, 9 Apr 2021 16:23:16 +0200 Subject: [PATCH 2/5] Remove unnecessary code --- genologics/descriptors.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/genologics/descriptors.py b/genologics/descriptors.py index 5b0235b..5a0b9bf 100644 --- a/genologics/descriptors.py +++ b/genologics/descriptors.py @@ -8,11 +8,6 @@ from genologics.constants import nsmap -try: - from urllib.parse import urlsplit, urlparse, parse_qs, urlunparse -except ImportError: - from urlparse import urlsplit, urlparse, parse_qs, urlunparse - from decimal import Decimal import datetime import time From 50cdb49e661920cac695f36d115db4d306641280 Mon Sep 17 00:00:00 2001 From: Steinar Sturlaugsson Date: Fri, 9 Apr 2021 16:23:37 +0200 Subject: [PATCH 3/5] Basestring is not required in python 3 --- genologics/descriptors.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/genologics/descriptors.py b/genologics/descriptors.py index 5a0b9bf..002b830 100644 --- a/genologics/descriptors.py +++ b/genologics/descriptors.py @@ -151,10 +151,7 @@ class UdfDictionary(object): "Dictionary-like container of UDFs, optionally within a UDT." def _is_string(self, value): - try: - return isinstance(value, basestring) - except: - return isinstance(value, str) + return isinstance(value, str) def __init__(self, instance, *args, **kwargs): self.instance = instance From 4cc8c061e83309b9b517149cc5ff9d60ff810abd Mon Sep 17 00:00:00 2001 From: Steinar Sturlaugsson Date: Fri, 9 Apr 2021 16:24:49 +0200 Subject: [PATCH 4/5] Use NotImplementedError instead of NotImplemented --- genologics/descriptors.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/genologics/descriptors.py b/genologics/descriptors.py index 002b830..c778462 100644 --- a/genologics/descriptors.py +++ b/genologics/descriptors.py @@ -263,14 +263,14 @@ def __setitem__(self, key, value): raise TypeError('URI UDF requires str or punycode (unicode) value') value = str(value) else: - raise NotImplemented("UDF type '%s'" % vtype) - if value is None: - node.text = '' - else: - if not isinstance(value, str): - if not self._is_string(value): - value = str(value).encode('UTF-8') - node.text = value + raise NotImplementedError("UDF type '%s'" % vtype) + if value is None: + node.text = '' + else: + if not isinstance(value, str): + if not self._is_string(value): + value = str(value).encode('UTF-8') + node.text = value break else: # Create new entry; heuristics for type if self._is_string(value): From d8a4f91d1500c3f9a435a7602f650b54e89dec87 Mon Sep 17 00:00:00 2001 From: Steinar Sturlaugsson Date: Fri, 9 Apr 2021 16:35:38 +0200 Subject: [PATCH 5/5] Refactor setitem --- genologics/descriptors.py | 92 +++++++++++++++++++++------------------ 1 file changed, 50 insertions(+), 42 deletions(-) diff --git a/genologics/descriptors.py b/genologics/descriptors.py index c778462..8f2d706 100644 --- a/genologics/descriptors.py +++ b/genologics/descriptors.py @@ -228,51 +228,59 @@ def __contains__(self, key): def __getitem__(self, key): return self._lookup[key] + def _validate_and_parse(self, vtype, value): + """ + Validates that the `value` is valid for vtype and returns a valid string representation + of the value that can be accepted by the API. + """ + if value is None: + value = '' + elif vtype == 'string' or vtype == 'str': + if not self._is_string(value): + raise TypeError('String UDF requires a string') + elif vtype == 'text': + if not self._is_string(value): + raise TypeError('Text UDF requires a string') + elif vtype == 'numeric': + if not isinstance(value, (int, float, Decimal)) and value != '': + raise TypeError('Numeric UDF requires int or float value') + else: + value = str(value) + elif vtype == 'boolean': + if not isinstance(value, bool): + raise TypeError('Boolean UDF requires bool value') + value = 'true' if value else 'false' + elif vtype == 'date': + if not isinstance(value, datetime.date): # Too restrictive? + raise TypeError('Date UDF requires datetime.date value') + value = str(value) + elif vtype == 'uri': + if not self._is_string(value): + raise TypeError('URI UDF requires a string') + value = str(value) + elif not isinstance(value, str): + if not self._is_string(value): + value = str(value).encode('UTF-8') + else: + raise NotImplementedError("UDF type '%s'" % vtype) + + return value + def __setitem__(self, key, value): self._lookup[key] = value + + found_node = None + for node in self._elems: - if node.attrib['name'] != key: continue - vtype = node.attrib['type'].lower() + if node.attrib['name'] != key: + continue + found_node = node - if value is None: - value='' - elif vtype == 'string': - if not self._is_string(value): - raise TypeError('String UDF requires str or unicode value') - elif vtype == 'str': - if not self._is_string(value): - raise TypeError('String UDF requires str or unicode value') - elif vtype == 'text': - if not self._is_string(value): - raise TypeError('Text UDF requires str or unicode value') - elif vtype == 'numeric': - if not isinstance(value, (int, float, Decimal)) and value != '': - raise TypeError('Numeric UDF requires int or float value') - else: - value = str(value) - elif vtype == 'boolean': - if not isinstance(value, bool): - raise TypeError('Boolean UDF requires bool value') - value = 'true' if value else 'false' - elif vtype == 'date': - if not isinstance(value, datetime.date): # Too restrictive? - raise TypeError('Date UDF requires datetime.date value') - value = str(value) - elif vtype == 'uri': - if not self._is_string(value): - raise TypeError('URI UDF requires str or punycode (unicode) value') - value = str(value) - else: - raise NotImplementedError("UDF type '%s'" % vtype) - if value is None: - node.text = '' - else: - if not isinstance(value, str): - if not self._is_string(value): - value = str(value).encode('UTF-8') - node.text = value - break - else: # Create new entry; heuristics for type + if found_node: + vtype = found_node.attrib['type'].lower() + found_node.text = self._validate_and_parse(vtype, value) + else: + # Create new entry; heuristics for type if self._is_string(value): vtype = 'Text' if '\n' in value else 'String' elif isinstance(value, bool): @@ -301,7 +309,7 @@ def __setitem__(self, key, value): elem.text = value - #update the internal elements and lookup with new values + # update the internal elements and lookup with new values self._update_elems() self._prepare_lookup()