From ddd6d1fe8a33ea29e37da901b5aa0271f9ccdd3a Mon Sep 17 00:00:00 2001
From: "i.nueske" <i.nueske@indiscale.com>
Date: Fri, 25 Oct 2024 10:48:22 +0200
Subject: [PATCH] MNT: Extended some comments, removed extra spaces, added some
ToDos
---
src/linkahead/apiutils.py | 85 ++++++++++++++++++++-------------------
1 file changed, 44 insertions(+), 41 deletions(-)
diff --git a/src/linkahead/apiutils.py b/src/linkahead/apiutils.py
index fab8c14f..d70f937f 100644
--- a/src/linkahead/apiutils.py
+++ b/src/linkahead/apiutils.py
@@ -183,7 +183,7 @@ def compare_entities(old_entity: Entity,
new_entity: Entity,
compare_referenced_records: bool = False
) -> tuple[dict[str, Any], dict[str, Any]]:
- """Compare two entites.
+ """Compare two entities.
The following attributes are compared:
- parents
@@ -200,7 +200,7 @@ def compare_entities(old_entity: Entity,
the following differs:
- datatype
- importance or
- - value (not implemented yet)
+ - value
In case of changed information the value listed under the respective key is the
value that is stored in the respective entity.
@@ -226,31 +226,33 @@ def compare_entities(old_entity: Entity,
newdiff: dict[str, Any] = {"properties": {}, "parents": []}
if old_entity is new_entity:
- return (olddiff, newdiff)
+ return olddiff, newdiff
if type(old_entity) is not type(new_entity):
raise ValueError(
"Comparison of different Entity types is not supported.")
- ### compare special attributes ###
+ # compare special attributes
for attr in SPECIAL_ATTRIBUTES:
if attr == "value":
continue
+
oldattr = old_entity.__getattribute__(attr)
- # we considere "" and None as nonexistent
+ # we consider "" and None to be nonexistent
old_entity_attr_na = (oldattr == "" or oldattr is None)
newattr = new_entity.__getattribute__(attr)
- # we considere "" and None as nonexistent
+ # we consider "" and None to be nonexistent
new_entity_attr_na = (newattr == "" or newattr is None)
- # both unset
+ # in both entities the current attribute is not set
if old_entity_attr_na and new_entity_attr_na:
continue
- # treat datatype separately: if datatype is an object on one side and string on the other.
+ # treat datatype separately if one datatype is an object and the other
+ # a string or int, and may be a name or id
if attr == "datatype":
- if (not old_entity_attr_na and not new_entity_attr_na):
+ if not old_entity_attr_na and not new_entity_attr_na:
if isinstance(oldattr, RecordType):
if oldattr.name == newattr:
continue
@@ -262,8 +264,7 @@ def compare_entities(old_entity: Entity,
if newattr.id == oldattr:
continue
-
- # only one set or different values
+ # add to diff if attribute has different values or is not set for one entity
if ((old_entity_attr_na ^ new_entity_attr_na)
or (oldattr != newattr)):
if not old_entity_attr_na:
@@ -272,27 +273,25 @@ def compare_entities(old_entity: Entity,
if not new_entity_attr_na:
newdiff[attr] = newattr
-
- # value
- if (old_entity.value != new_entity.value):
- # basic comparison of value objects says they are different
+ # compare value
+ if old_entity.value != new_entity.value:
+ # though the values are not equal, they might be equivalent:
same_value = False
if compare_referenced_records:
- # scalar reference
+ # both values are scalar references:
if isinstance(old_entity.value, Entity) and isinstance(new_entity.value, Entity):
- # explicitely not recursive to prevent infinite recursion
+ # compare_referenced_records=False to prevent infinite recursion
same_value = empty_diff(
old_entity.value, new_entity.value, compare_referenced_records=False)
- # list of references
+ # both values are a list of references:
elif isinstance(old_entity.value, list) and isinstance(new_entity.value, list):
- # all elements in both lists actually are entity objects
- # TODO: check, whether mixed cases can be allowed or should lead to an error
+ # if all elements in both lists are entity objects, check each pair for equality
+ # TODO: check whether mixed cases should be allowed or lead to an error
if (all([isinstance(x, Entity) for x in old_entity.value])
and all([isinstance(x, Entity) for x in new_entity.value])):
- # can't be the same if the lengths are different
+ # lists can't be the same if the lengths are different
if len(old_entity.value) == len(new_entity.value):
- # do a one-by-one comparison:
- # the values are the same if all diffs are empty
+ # the lists are the same if the diffs of each entry pair are empty
same_value = all(
[empty_diff(x, y, False) for x, y
in zip(old_entity.value, new_entity.value)])
@@ -301,21 +300,24 @@ def compare_entities(old_entity: Entity,
olddiff["value"] = old_entity.value
newdiff["value"] = new_entity.value
- # properties
+ # compare properties
for prop in old_entity.properties:
matching = [p for p in new_entity.properties if p.name.lower() == prop.name.lower()]
if len(matching) == 0:
- # old has prop and new does not
- olddiff["properties"][prop.name] = {}
+ # old_entity has prop, new_entity does not
+ olddiff["properties"][prop.name] = {}
elif len(matching) ==1:
- olddiff["properties"][prop.name] = {}
- newdiff["properties"][prop.name] = {}
+ olddiff["properties"][prop.name] = {}
+ newdiff["properties"][prop.name] = {}
oldpropdiff = olddiff["properties"][prop.name]
newpropdiff = newdiff["properties"][prop.name]
- # use compare function detect difference of properties
+ # recursive call to determine the differences between properties
+ # ToDo: This can lead to infinite recursion if two properties have
+ # each other as subproperties
od, nd = compare_entities(prop, matching[0], compare_referenced_records=compare_referenced_records)
- # we do not care about parents and properties here # TODO do we?
+ # as we do not care about parents and properties here, discard their entries
+ # TODO do we?
od.pop("parents")
od.pop("properties")
nd.pop("parents")
@@ -324,27 +326,28 @@ def compare_entities(old_entity: Entity,
oldpropdiff.update(od)
newpropdiff.update(nd)
- # importance is associated with the record. So do it extra
- if (old_entity.get_importance(prop.name) != new_entity.get_importance(prop.name)):
- oldpropdiff["importance"] = old_entity.get_importance(prop.name)
- newpropdiff["importance"] = new_entity.get_importance(prop.name)
+ # As the importance of a property is an attribute of the record and not
+ # the property, it is not contained in the diff returned by compare_entities
+ if old_entity.get_importance(prop.name) != new_entity.get_importance(prop.name):
+ oldpropdiff["importance"] = old_entity.get_importance(prop.name)
+ newpropdiff["importance"] = new_entity.get_importance(prop.name)
- # in case there was actually no difference, we remove the dict keys again
- if (len(newpropdiff) == 0 and len(oldpropdiff) == 0):
+ # in case there is no difference, we remove the dict keys again
+ if len(newpropdiff) == 0 and len(oldpropdiff) == 0:
newdiff["properties"].pop(prop.name)
olddiff["properties"].pop(prop.name)
else:
raise NotImplementedError(
"Comparison not implemented for multi-properties.")
- # add the properties that are only present in new
+ # we have not yet compared properties that do not exist in old_entity
for prop in new_entity.properties:
if len([0 for p in old_entity.properties if p.name.lower() == prop.name.lower()]) == 0:
newdiff["properties"][prop.name] = {}
- # parents
-
- # TODO we only use names for parents and property matching
+ # compare parents
+ # ToDo: Compare using filter function, compare inheritance level for RTs
+ # TODO we currently only use names for parents and property matching
for parent in old_entity.parents:
if len([0 for p in new_entity.parents if p.name.lower() == parent.name.lower()]) == 0:
olddiff["parents"].append(parent.name)
@@ -353,7 +356,7 @@ def compare_entities(old_entity: Entity,
if len([0 for p in old_entity.parents if p.name.lower() == parent.name.lower()]) == 0:
newdiff["parents"].append(parent.name)
- return (olddiff, newdiff)
+ return olddiff, newdiff
def empty_diff(old_entity: Entity, new_entity: Entity,
--
GitLab