diff --git a/.gitlab/merge_request_templates/General.md b/.gitlab/merge_request_templates/General.md deleted file mode 100644 index b3934b37072ff377f46028ca8a93cebe3771b6fd..0000000000000000000000000000000000000000 --- a/.gitlab/merge_request_templates/General.md +++ /dev/null @@ -1,29 +0,0 @@ -Insert a meaningful description for this merge request here. What is the -new/changed behavior? Which bug was fixed? - ---- - -Add the text above this line to the git comment. - -# Check list for the author # - -- [ ] All tests pass. (style, unit/integration, static code analysis) -- [ ] What is the main focus for the reviewer? What is the core intent of this - MR? -- [ ] How to set up a test environment. -- [ ] Annotations in code (Gitlab comments) - - Intent of new code - - Problems with old code - - Why this implementation? - -# Check list for the reviewer # - -- [ ] Tests pass -- [ ] Do I understand the intent of this MR? -- [ ] Does the test environment setup work? -- [ ] Is the intended behavior reproducible in the test environment? -- [ ] Code overview: Which functions/classes are relevant for understanding the MR? -- [ ] Work through the open comment threads. -- [ ] Are the doc strings / comments up-to-date? Or have there been behavioral - changes without amending the documentation? -- [ ] Are there spezifications? Are they satisfied? diff --git a/CHANGELOG.md b/CHANGELOG.md index 76ca2594102b4a455712eef30b0190aa3f45e7e9..73403111ac141553237a3f1691adc22ad2183a16 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * `get_property` method also accepts instances of properties now, e.g. `record.get_property(Property(name="length"))` +* the value of properties is parsed to python types (int, float, boolean) when + setting the value with the setter and when the datatype changes. Before this + change, the value was parsed to python types only when parsing an xml and + only for int and float. ### Deprecated ### diff --git a/src/caosdb/common/models.py b/src/caosdb/common/models.py index 343b0bbbeea8605361422120a4ff2aae445a59e8..5472cf091c3b9ce80f2be3a6499b0314f8073022 100644 --- a/src/caosdb/common/models.py +++ b/src/caosdb/common/models.py @@ -82,27 +82,29 @@ class Entity(object): def __init__(self, name=None, id=None, description=None, # @ReservedAssignment datatype=None, value=None, **kwargs): self.__role = kwargs["role"] if "role" in kwargs else None - self.name = name - self.description = description - self.id = id - self.value = value + self._checksum = None + self._size = None + self._upload = None + self._wrapped_entity = None + self._cuid = None + self._flags = dict() + self.__value = None + self.__datatype = None self.datatype = datatype + self.value = value self.messages = _Messages() self.properties = _Properties() self.parents = _Parents() self.path = None self.file = None - self._checksum = None - self._size = None - self._upload = None self.unit = None - self._cuid = None self.acl = None self.permissions = None - self._wrapped_entity = None - self._flags = dict() self.is_valid = lambda: False self.is_deleted = lambda: False + self.name = name + self.description = description + self.id = id @property def role(self): @@ -159,6 +161,8 @@ class Entity(object): @datatype.setter def datatype(self, new_type): + # re-parse value + self.__value = _parse_value(new_type, self.__value) self.__datatype = new_type @property @@ -196,7 +200,7 @@ class Entity(object): @value.setter def value(self, new_value): - self.__value = new_value + self.__value = _parse_value(self.datatype, new_value) @property def path(self): @@ -820,22 +824,11 @@ class Entity(object): Was ' + str(type(child))) # parse VALUE - if len(vals): - entity.value = _parse_col_values(entity.datatype, vals) - elif elem.text is not None: - if elem.text.strip() != "": - text_val = elem.text.strip() - - if entity.datatype == DOUBLE: - entity.value = float(text_val) - elif entity.datatype == DATETIME or entity.datatype == TEXT: - entity.value = text_val - else: - try: # for references and integer - entity.value = int(text_val) - except BaseException: - entity.value = text_val + # The value[s] have been inside a <Value> tag. + entity.value = vals + elif elem.text is not None and elem.text.strip() != "": + entity.value = elem.text.strip() return entity @@ -981,35 +974,66 @@ class Entity(object): return self -def _parse_col_values(cdt, vals): - matcher = re.compile(r"^(?P<col>[^<]+)<(?P<dt>[^>]+)>$") - m = matcher.match(cdt) - if m: - col = m.group("col") - dt = m.group("dt") - - if col == "LIST": - ret = list() - add = ret.append +def _parse_value(datatype, value): + if value is None: + return value + if datatype is None: + return value + if datatype == DOUBLE: + return float(value) + if datatype == INTEGER: + return int(str(value)) + if datatype == BOOLEAN: + if str(value).lower() == "true": + return True + elif str(value).lower() == "false": + return False else: - return vals + raise ValueError("Boolean value was {}.".format(value)) + if datatype in [DATETIME, TEXT]: + if isinstance(value, str): + return value + + # deal with collections + if isinstance(datatype, str): + matcher = re.compile(r"^(?P<col>[^<]+)<(?P<dt>[^>]+)>$") + m = matcher.match(datatype) + if m: + col = m.group("col") + dt = m.group("dt") + + if col == "LIST": + ret = list() + else: + return value - for v in vals: - if dt == DOUBLE: - add(float(v)) - elif dt == TEXT or dt == DATETIME: - add(v) + if hasattr(value, "__iter__") and not isinstance(value, str): + for v in value: + ret.append(_parse_value(dt, v)) else: - try: - add(int(v)) - except (ValueError, TypeError): - add(v) + # put a single value into a list since the datatype says so. + ret.append(_parse_value(dt, value)) - return ret + return ret - if len(vals) == 1: - return vals[0] - return vals + # This is for a special case, where the xml parser could not differentiate + # between single values and lists with one element. As + if hasattr(value, "__len__") and len(value) == 1: + return _parse_value(datatype, value[0]) + + # deal with references + if isinstance(value, Entity): + return value + if isinstance(value, str) and "@" in value: + # probably this is a versioned reference + return str(value) + else: + # for unversioned references + try: + return int(value) + except ValueError: + # reference via name + return str(value) def _log_request(request, xml_body=None): diff --git a/unittests/test_add_property.py b/unittests/test_add_property.py index 917ee68b5ae3d8ce738c8dff6dbfdfed9fb75580..1f8c7e3b832c701f114d1014d8bc9f9409329e80 100644 --- a/unittests/test_add_property.py +++ b/unittests/test_add_property.py @@ -189,7 +189,7 @@ def test_property_parameter_with_entity_and_datatype(): description="This is the length of something.") assert 0 == len(rec.get_properties()) - rec.add_property(abstract_property, 3.14, datatype=db.INTEGER) + rec.add_property(abstract_property, 300, datatype=db.INTEGER) assert 1 == len(rec.get_properties()) concrete_property = rec.get_property("length") assert concrete_property is not None @@ -197,9 +197,29 @@ def test_property_parameter_with_entity_and_datatype(): assert concrete_property.id == 512 assert concrete_property.description == "This is the length of something." assert concrete_property.unit == "m" - assert concrete_property.value == 3.14 + assert concrete_property.value == 300 assert concrete_property.datatype == db.INTEGER - assert concrete_property._wrapped_entity == abstract_property + assert id(concrete_property._wrapped_entity) == id(abstract_property) + + concrete_property.value = None + + with raises(ValueError): + # cannot parse 3.14 to integer + concrete_property.value = 3.14 + + assert concrete_property.value is None + assert concrete_property.datatype == db.INTEGER + + concrete_property.datatype = db.DOUBLE + concrete_property.value = 3.14 + + with raises(ValueError): + # cannot parse 3.14 to integer + concrete_property.datatype = db.INTEGER + + # nothing should've changed after the ValueError + assert concrete_property.datatype == db.DOUBLE + assert concrete_property.value == 3.14 def test_kw_name_and_value(): diff --git a/unittests/test_datatype.py b/unittests/test_datatype.py index 7cc5fd41c74ece160cac44b8e4386061fefd244f..9b3c6267fb018e2cd3085dea568d7396c4549ac8 100644 --- a/unittests/test_datatype.py +++ b/unittests/test_datatype.py @@ -18,9 +18,10 @@ # along with this program. If not, see <https://www.gnu.org/licenses/>. # # ** end header - +from pytest import raises import caosdb as db from caosdb.common import datatype +from caosdb.common.models import _parse_value def test_list(): @@ -32,3 +33,57 @@ def test_list_utilites(): """Test for example if get_list_datatype works.""" dtype = db.LIST(db.INTEGER) assert datatype.get_list_datatype(dtype) == db.INTEGER + + +def test_parsing_of_intger_list_values(): + dtype = db.LIST(db.INTEGER) + assert _parse_value(dtype, [1, 2, 3]) == [1, 2, 3] + assert _parse_value(dtype, [1]) == [1] + assert _parse_value(dtype, [None, 1, 2, 3]) == [None, 1, 2, 3] + assert _parse_value(dtype, [1, None, 1, 2, 3]) == [1, None, 1, 2, 3] + assert _parse_value(dtype, ["4", 4]) == [4, 4] + assert _parse_value(dtype, []) == [] + assert _parse_value(dtype, None) is None + assert _parse_value(None, [1, 2, 3.14, "asdf"]) == [1, 2, 3.14, "asdf"] + assert _parse_value(dtype, 1) == [1] + + with raises(ValueError): + # float value in list + _parse_value(dtype, ["4.3", 4]) + + +def test_parsing_of_boolean_list_values(): + dtype = db.LIST(db.BOOLEAN) + assert _parse_value(dtype, [True, False, True]) == [True, False, True] + assert _parse_value(dtype, ["true", False, None]) == [True, False, None] + + with raises(ValueError): + _parse_value(dtype, ["not a boolean"]) + + +def test_parsing_of_unknown_col_datatype(): + dtype = "Arraay<TEXT>" + obj = {"a": "b"} + assert id(_parse_value(dtype, obj)) == id(obj) + + +def test_parsing_of_references(): + dtype = "Person" + assert _parse_value(dtype, "Anna Lytik") == "Anna Lytik" + assert _parse_value(None, "Anna Lytik") == "Anna Lytik" + assert _parse_value(dtype, "2345@sdfg") == "2345@sdfg" + assert _parse_value(dtype, "2345") == 2345 + assert _parse_value(dtype, 2345) == 2345 + + entity = db.Record(name="bla") + assert id(_parse_value(dtype, entity)) == id(entity) + + dtype = db.RecordType(name="Person") + assert _parse_value(dtype, "Anna Lytik") == "Anna Lytik" + assert _parse_value(None, "Anna Lytik") == "Anna Lytik" + assert _parse_value(dtype, "2345@sdfg") == "2345@sdfg" + assert _parse_value(dtype, "2345") == 2345 + assert _parse_value(dtype, 2345) == 2345 + + entity = db.Record(name="bla") + assert id(_parse_value(dtype, entity)) == id(entity) diff --git a/unittests/test_property.py b/unittests/test_property.py index 7d8d238edd59e47707f6e1ad831b0bb640cf0a6d..8b2deeb4b06cfbf3d42881201307a1e2122124e3 100644 --- a/unittests/test_property.py +++ b/unittests/test_property.py @@ -5,6 +5,8 @@ # # Copyright (C) 2018 Research Group Biomedical Physics, # Max-Planck-Institute for Dynamics and Self-Organization Göttingen +# Copyright (C) 2020 IndiScale GmbH <info@indiscale.com> +# Copyright (C) 2020 Timm Fitschen <t.fitschen@indiscale.com> # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU Affero General Public License as