Merge branch 'revert-5ce8e910' into 'dev'

Revert "Merge branch 'f-check-merge-entities' into 'dev'"

See merge request !105

CHANGELOG.md

@@ -22,11 +22,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Removed ###
 
 ### Fixed ###
-
 - Fixed `src/caosdb/utils/checkFileSystemConsistency.py`
-- `compare_entities` now corretly raises a `NotImplementedError` in case of
-  comparing two `Property` entities or in case of entities with multi
-  properties.
 
 ### Security ###
 

src/caosdb/apiutils.py

@@ -178,8 +178,7 @@ def getCommitIn(folder):
     return get_commit_in(folder)
 
 
-def compare_entities(old_entity: Entity, new_entity: Entity,
-                     compare_referenced_records: bool = False):
+def compare_entities(old_entity: Entity, new_entity: Entity, compare_referenced_records: bool = False):
     """Compare two entites.
 
     Return a tuple of dictionaries, the first index belongs to additional information for old
@@ -190,9 +189,8 @@ def compare_entities(old_entity: Entity, new_entity: Entity,
     - Information about properties:
       - Each property lists either an additional property or a property with a changed:
         - datatype
-        - unit
         - importance or
-        - value
+        - value (not implemented yet)
 
         In case of changed information the value listed under the respective key shows the
         value that is stored in the respective entity.
@@ -202,9 +200,6 @@ def compare_entities(old_entity: Entity, new_entity: Entity,
     `compare_referenced_records = False` to prevent infinite recursion in case
     of circular references).
 
-    NOTE: This function does not work for abstract properties! I.e. it is not possible
-          to directly compare two entities that are of class caosdb.Property.
-
     Parameters
     ----------
     old_entity, new_entity : Entity
@@ -217,12 +212,6 @@ def compare_entities(old_entity: Entity, new_entity: Entity,
         identical records are stored in different objects. Default is False.
 
     """
-
-    for entity in (old_entity, new_entity):
-        if isinstance(entity, Property):
-            raise NotImplementedError("The function compare_entities does not work for "
-                                      "comparing abstract properties.")
-
     olddiff: Dict[str, Any] = {"properties": {}, "parents": []}
     newdiff: Dict[str, Any] = {"properties": {}, "parents": []}
 
@@ -250,7 +239,7 @@ def compare_entities(old_entity: Entity, new_entity: Entity,
         if not old_entity_attr_exists and not new_entity_attr_exists:
             continue
 
-        if ((old_entity_attr_exists != new_entity_attr_exists)
+        if ((old_entity_attr_exists ^ new_entity_attr_exists)
                 or (oldattr != newattr)):
 
             if old_entity_attr_exists:
@@ -262,21 +251,9 @@ def compare_entities(old_entity: Entity, new_entity: Entity,
     # properties
 
     for prop in old_entity.properties:
-        # Find the corresponding property in new_entity:
         matching = [p for p in new_entity.properties if p.name == prop.name]
 
-        # This is needed for checking for multi properties in old_entity:
-        # TODO: is there a better way?
-        matching_old = [p for p in old_entity.properties if p.name == prop.name]
-        if len(matching_old) != 1:
-            raise NotImplementedError(
-                "Comparison not implemented for multi-properties.")
-        if len(matching) > 1:
-            raise NotImplementedError(
-                "Comparison not implemented for multi-properties.")
-
         if len(matching) == 0:
-            # There is no matching property in new_entity:
             olddiff["properties"][prop.name] = {}
         elif len(matching) == 1:
             newdiff["properties"][prop.name] = {}
@@ -306,7 +283,6 @@ def compare_entities(old_entity: Entity, new_entity: Entity,
                     # scalar reference
                     if isinstance(prop.value, Entity) and isinstance(matching[0].value, Entity):
                         # explicitely not recursive to prevent infinite recursion
-                        # TODO: why not use a recursion detection with a cache?
                         same_value = empty_diff(
                             prop.value, matching[0].value, compare_referenced_records=False)
                     # list of references
@@ -330,14 +306,11 @@ def compare_entities(old_entity: Entity, new_entity: Entity,
                 newdiff["properties"].pop(prop.name)
                 olddiff["properties"].pop(prop.name)
 
-    # Check whether there are missing properties in old_entity, additionally
-    # check for multi-properties that are currently not supported:
-    for prop in new_entity.properties:
-        matching = [p for p in new_entity.properties if p.name == prop.name]
-        if len(matching) > 1:
+        else:
             raise NotImplementedError(
                 "Comparison not implemented for multi-properties.")
 
+    for prop in new_entity.properties:
         if len([0 for p in old_entity.properties if p.name == prop.name]) == 0:
             newdiff["properties"][prop.name] = {}
 

unittests/test_apiutils.py

@@ -101,15 +101,14 @@ def test_compare_entities():
     r2 = db.Record()
     r1.add_parent("bla")
     r2.add_parent("bla")
-    r1.add_parent("lopp")  # r1 has one additional parent
+    r1.add_parent("lopp")
     r1.add_property("test", value=2)
-    r2.add_property("test", value=2)  # a property with the same value
+    r2.add_property("test", value=2)
     r1.add_property("tests", value=3)
-    r2.add_property("tests", value=45)  # a property with different value
+    r2.add_property("tests", value=45)
     r1.add_property("tester", value=3)
-    r2.add_property("tester", )  # a property where r2 has no value
+    r2.add_property("tester", )
     r1.add_property("tests_234234", value=45)
-    # one additional property that the other one does not have for both
     r2.add_property("tests_TT", value=45)
 
     diff_r1, diff_r2 = compare_entities(r1, r2)
@@ -131,33 +130,6 @@ def test_compare_entities():
     assert "tests_234234" in diff_r1["properties"]
     assert "tests_TT" in diff_r2["properties"]
 
-    # Check the value:
-    assert diff_r1["properties"]["tests"]["value"] == 3
-    assert diff_r2["properties"]["tests"]["value"] == 45
-
-
-def test_compare_equality():
-    r1 = db.Record()
-    r2 = db.Record()
-    diff_r1, diff_r2 = compare_entities(r1, r2)
-    for i in ("parents", "properties"):
-        assert len(diff_r1[i]) == 0
-        assert len(diff_r2[i]) == 0
-    assert empty_diff(r1, r2)
-
-    r1.add_parent("bla")
-    r2.add_parent("bla")
-    assert empty_diff(r1, r2)
-
-    r1.add_property("test", value=2)
-    r2.add_property("test", value=2)  # a property with the same value
-    diff_r1, diff_r2 = compare_entities(r1, r2)
-    assert empty_diff(r1, r2)
-
-    r1.add_parent("blobb")
-    r2.add_parent("blobb")
-    assert empty_diff(r1, r2)
-
 
 def test_compare_entities_units():
     r1 = db.Record()
@@ -193,10 +165,6 @@ def test_compare_entities_units():
     assert diff_r1["properties"]["test"]["unit"] == "cm"
     assert diff_r2["properties"]["test"]["unit"] == "m"
 
-    # Check the value:
-    assert diff_r1["properties"]["tests"]["value"] == 3
-    assert diff_r2["properties"]["tests"]["value"] == 45
-
 
 def test_compare_special_properties():
     # Test for all known special properties:
@@ -244,114 +212,46 @@ def test_compare_special_properties():
         assert len(diff_r2["properties"]) == 0
 
 
-def test_compare_importances():
-    r1 = db.Record()
-    r2 = db.Record()
-    r1.add_property("test", value=2, unit="cm", importance="SUGGESTED")
-    r2.add_property("test", value=2, unit="cm")
-    assert not empty_diff(r1, r2)
-    diff_r1, diff_r2 = compare_entities(r1, r2)
-    assert diff_r1["properties"]["test"]["importance"] == "SUGGESTED"
-    assert diff_r2["properties"]["test"]["importance"] == "FIX"
-    for diff in (diff_r1, diff_r2):
-        assert len(diff["properties"]["test"]) == 1
-        assert len(diff["parents"]) == 0
-
-
+@pytest.mark.xfail
 def test_compare_properties():
     p1 = db.Property()
     p2 = db.Property()
 
-    with pytest.raises(NotImplementedError, match=".*abstract properties.*"):
-        compare_entities(p1, p2)
-
-
-def test_multi_properties():
-    # This test is rather lengthy, because:
-    # - previously the check for multi-properties was only implemented for the
-    #   new_entity paramter of the function.
-    # - Because of the API of pylib the behavior depended on the order of adding the
-    #   properties to the records.
-
-    r1 = db.Record()
-    r2 = db.Record()
-    r1.add_property("test", value=2)
-    r1.add_property("test", value=4)
-    r2.add_property("test", value=2)
-    # That would be expected:
-    # assert not empty_diff(r1, r2)
-    with pytest.raises(NotImplementedError, match=".*multi-properties.*"):
-        compare_entities(r1, r2)
-
-    r1 = db.Record()
-    r2 = db.Record()
-    r1.add_property("test", value=4)
-    r1.add_property("test", value=2)
-    r2.add_property("test", value=2)
-    # That would be expected:
-    # assert not empty_diff(r1, r2)
-    with pytest.raises(NotImplementedError, match=".*multi-properties.*"):
-        compare_entities(r1, r2)
-
-    r1 = db.Record()
-    r2 = db.Record()
-    r1.add_property("test", value=4)
-    r1.add_property("test", value=2)
-    r2.add_property("test", value=2)
-    r2.add_property("test", value=4)
-    # That would be expected:
-    # assert empty_diff(r1, r2)
-    with pytest.raises(NotImplementedError, match=".*multi-properties.*"):
-        compare_entities(r1, r2)
-
-    r1 = db.Record()
-    r2 = db.Record()
-    r1.add_property("test", value=4)
-    r2.add_property("test", value=4)
-    r2.add_property("test", value=2)
-    # That would be expected:
-    # assert not empty_diff(r1, r2)
-    with pytest.raises(NotImplementedError, match=".*multi-properties.*"):
-        compare_entities(r1, r2)
-
-    r1 = db.Record()
-    r2 = db.Record()
-    r1.add_property("test", value=4)
-    r2.add_property("test", value=2)
-    r2.add_property("test", value=4)
-    # That would be expected:
-    # assert not empty_diff(r1, r2)
-    with pytest.raises(NotImplementedError, match=".*multi-properties.*"):
-        compare_entities(r1, r2)
-
-    r1 = db.Record()
-    r2 = db.Record()
-    r1.add_property("test", value=4)
-    r1.add_property("test", value=2)
-    r2.add_property("test", value=2)
-    r2.add_property("test", value=5)
-    # That would be expected:
-    # assert empty_diff(r1, r2)
-    with pytest.raises(NotImplementedError, match=".*multi-properties.*"):
-        compare_entities(r1, r2)
-
-    r1 = db.Record()
-    r2 = db.Record()
-    r1.add_property("test", value=4)
-    r1.add_property("test", value=2)
-    # That would be expected:
-    # assert empty_diff(r1, r2)
-    with pytest.raises(NotImplementedError, match=".*multi-properties.*"):
-        compare_entities(r1, r2)
+    diff_r1, diff_r2 = compare_entities(p1, p2)
+    assert len(diff_r1["parents"]) == 0
+    assert len(diff_r2["parents"]) == 0
+    assert len(diff_r1["properties"]) == 0
+    assert len(diff_r2["properties"]) == 0
 
-    r1 = db.Record()
-    r2 = db.Record()
-    r2.add_property("test", value=2)
-    r2.add_property("test", value=5)
-    # That would be expected:
-    # assert empty_diff(r1, r2)
-    with pytest.raises(NotImplementedError, match=".*multi-properties.*"):
-        compare_entities(r1, r2)
+    p1.importance = "SUGGESTED"
+    diff_r1, diff_r2 = compare_entities(p1, p2)
+    assert len(diff_r1["parents"]) == 0
+    assert len(diff_r2["parents"]) == 0
+    assert len(diff_r1["properties"]) == 0
+    assert len(diff_r2["properties"]) == 0
+    assert "importance" in diff_r1
+    assert diff_r1["importance"] == "SUGGESTED"
+
+    # TODO: I'm not sure why it is not like this:
+    # assert diff_r2["importance"] is None
+    # ... but:
+    assert "importance" not in diff_r2
+
+    p2.importance = "SUGGESTED"
+    p1.value = 42
+    p2.value = 4
+
+    diff_r1, diff_r2 = compare_entities(p1, p2)
+    assert len(diff_r1["parents"]) == 0
+    assert len(diff_r2["parents"]) == 0
+    assert len(diff_r1["properties"]) == 0
+    assert len(diff_r2["properties"]) == 0
+
+    # Comparing values currently does not seem to be implemented:
+    assert "value" in diff_r1
+    assert diff_r1["value"] == 42
+    assert "value" in diff_r2
+    assert diff_r2["value"] == 4
 
 
 def test_copy_entities():