Merge pull request #275 from mpsonntag/refMerge

Refactor Property and Section merge

Author: jgrewe

odml/base.py

@@ -84,34 +84,18 @@ def __hash__(self):
         return id(self)
 
 
-class SafeList(list):
+class SmartList(list):
 
-    def index(self, obj):
+    def __init__(self, content_type):
         """
-        Find obj in list
-
-        Be sure to use "is" based comparison (instead of __eq__)
+        Only values of the instance *content_type* can be added to the SmartList.
         """
-        for i, e in enumerate(self):
-            if e is obj:
-                return i
-        raise ValueError("remove: %s not in list" % repr(obj))
-
-    def remove(self, obj):
-        """
-        Remove an element from this list.
-
-        Be sure to use "is" based comparison (instead of __eq__)
-        """
-        del self[self.index(obj)]
-
-
-class SmartList(SafeList):
+        self._content_type = content_type
+        super(SmartList, self).__init__()
 
     def __getitem__(self, key):
         """
-        Provides element index also by searching for an element with a given
-        name
+        Provides element index also by searching for an element with a given name.
         """
         # Try normal list index first (for integers)
         if isinstance(key, int):
@@ -125,31 +109,68 @@ def __getitem__(self, key):
         # and fail eventually
         raise KeyError(key)
 
+    def __setitem__(self, key, value):
+        """
+        Replaces item at list[*key*] with *value*.
+        :param key: index position
+        :param value: object that replaces item at *key* position.
+                      value has to be of the same content type as the list.
+                      In this context usually a Section or a Property.
+        """
+        if not isinstance(value, self._content_type):
+            raise ValueError("List only supports elements of type '%s'" %
+                             self._content_type)
+
+        # If required remove new object from its old parents child-list
+        if hasattr(value, "_parent") and (value._parent and value in value._parent):
+            value._parent.remove(value)
+
+        # If required move parent reference from replaced to new object
+        # and set parent reference on replaced object None.
+        if hasattr(self[key], "_parent"):
+            value._parent = self[key]._parent
+            self[key]._parent = None
+
+        super(SmartList, self).__setitem__(key, value)
+
     def __contains__(self, key):
         for obj in self:
             if (hasattr(obj, "name") and obj.name == key) or key == obj:
                 return True
 
+    def index(self, obj):
+        """
+        Find obj in list
+        """
+        for i, e in enumerate(self):
+            if e is obj:
+                return i
+        raise ValueError("remove: %s not in list" % repr(obj))
+
+    def remove(self, obj):
+        """
+        Remove an element from this list.
+        """
+        del self[self.index(obj)]
+
     def append(self, *obj_tuple):
-        from odml.section import BaseSection
-        from odml.doc import BaseDocument
         for obj in obj_tuple:
             if obj.name in self:
                 raise KeyError(
                     "Object with the same name already exists! " + str(obj))
 
-            if (not isinstance(obj, BaseSection)) & \
-               isinstance(self, BaseDocument):
-                raise KeyError("Object " + str(obj) + " is not a Section.")
+            if not isinstance(obj, self._content_type):
+                raise ValueError("List only supports elements of type '%s'" %
+                                 self._content_type)
 
             super(SmartList, self).append(obj)
 
 
 @allow_inherit_docstring
 class sectionable(baseobject):
-
     def __init__(self):
-        self._sections = SmartList()
+        from odml.section import Section
+        self._sections = SmartList(Section)
         self._repository = None
 
     @property
@@ -518,9 +539,10 @@ def clone(self, children=True):
         Clone this object recursively allowing to copy it independently
         to another document
         """
+        from odml.section import Section
         obj = super(sectionable, self).clone(children)
         obj._parent = None
-        obj._sections = SmartList()
+        obj._sections = SmartList(Section)
         if children:
             for s in self._sections:
                 obj.append(s.clone())

odml/property.py

@@ -373,45 +373,83 @@ def clone(self):
 
         return obj
 
-    def merge(self, other, strict=True):
+    def merge_check(self, source, strict=True):
         """
-        Merges the property 'other' into self, if possible. Information
-        will be synchronized. Method will raise a ValueError when the
-        information in this property and the passed property are in
-        conflict.
+        Checks whether a source Property can be merged with self as destination and
+        raises a ValueError if the values of source and destination are not compatible.
+        With parameter *strict=True* a ValueError is also raised, if any of the
+        attributes unit, definition, uncertainty, reference or value_origin and dtype
+        differ in source and destination.
 
-        :param other: an odML Property.
-        :param strict: Bool value to indicate whether types should be implicitly converted
-               even when information may be lost. Default is True, i.e. no conversion,
-               and a ValueError will be raised if types do not match.
+        :param source: an odML Property.
+        :param strict: If True, the attributes dtype, unit, uncertainty, definition,
+                       reference and value_origin of source and destination
+                       must be identical.
         """
-        assert(isinstance(other, BaseProperty))
-        if strict and self.dtype != other.dtype:
+        if not isinstance(source, BaseProperty):
+            raise ValueError("odml.Property.merge: odML Property required.")
+
+        # Catch unmerge-able values at this point to avoid
+        # failing Section tree merges which cannot easily be rolled back.
+        new_value = self._convert_value_input(source.value)
+        if not self._validate_values(new_value):
+            raise ValueError("odml.Property.merge: passed value(s) cannot "
+                             "be converted to data type '%s'!" % self._dtype)
+        if not strict:
+            return
+
+        if (self.dtype is not None and source.dtype is not None and
+                self.dtype != source.dtype):
             raise ValueError("odml.Property.merge: src and dest dtypes do not match!")
 
-        if self.unit is not None and other.unit is not None and self.unit != other.unit:
-            raise ValueError("odml.Property.merge: src and dest units (%s, %s) do not match!" % (other.unit, self.unit))
+        if self.unit is not None and source.unit is not None and self.unit != source.unit:
+            raise ValueError("odml.Property.merge: "
+                             "src and dest units (%s, %s) do not match!" %
+                             (source.unit, self.unit))
 
-        if self.definition is not None and other.definition is not None:
+        if (self.uncertainty is not None and source.uncertainty is not None and
+                self.uncertainty != source.uncertainty):
+            raise ValueError("odml.Property.merge: "
+                             "src and dest uncertainty both set and do not match!")
+
+        if self.definition is not None and source.definition is not None:
             self_def = ''.join(map(str.strip, self.definition.split())).lower()
-            other_def = ''.join(map(str.strip, other.definition.split())).lower()
+            other_def = ''.join(map(str.strip, source.definition.split())).lower()
             if self_def != other_def:
-                raise ValueError("odml.Property.merge: src and dest definitions do not match!")
-
-        if self.uncertainty is not None and other.uncertainty is not None:
-            raise ValueError("odml.Property.merge: src and dest uncertainty both set and do not match!")
+                raise ValueError("odml.Property.merge: "
+                                 "src and dest definitions do not match!")
 
-        if self.reference is not None and other.reference is not None:
+        if self.reference is not None and source.reference is not None:
             self_ref = ''.join(map(str.strip, self.reference.lower().split()))
-            other_ref = ''.join(map(str.strip, other.reference.lower().split()))
+            other_ref = ''.join(map(str.strip, source.reference.lower().split()))
             if self_ref != other_ref:
-                raise ValueError("odml.Property.merge: src and dest references are in conflict!")
+                raise ValueError("odml.Property.merge: "
+                                 "src and dest references are in conflict!")
 
-        if self.value_origin is not None and other.value_origin is not None:
+        if self.value_origin is not None and source.value_origin is not None:
             self_ori = ''.join(map(str.strip, self.value_origin.lower().split()))
-            other_ori = ''.join(map(str.strip, other.value_origin.lower().split()))
+            other_ori = ''.join(map(str.strip, source.value_origin.lower().split()))
             if self_ori != other_ori:
-                raise ValueError("odml.Property.merge: src and dest value_origin are in conflict!")
+                raise ValueError("odml.Property.merge: "
+                                 "src and dest value_origin are in conflict!")
+
+    def merge(self, other, strict=True):
+        """
+        Merges the Property 'other' into self, if possible. Information
+        will be synchronized. By default the method will raise a ValueError when the
+        information in this property and the passed property are in conflict.
+
+        :param other: an odML Property.
+        :param strict: Bool value to indicate whether types should be implicitly converted
+               even when information may be lost. Default is True, i.e. no conversion,
+               and a ValueError will be raised if types or other attributes do not match.
+               If a conflict arises with strict=False, the attribute value of self will
+               be kept, while the attribute value of other will be lost.
+        """
+        if not isinstance(other, BaseProperty):
+            raise TypeError("odml.Property.merge: odml Property required.")
+
+        self.merge_check(other, strict)
 
         if self.value_origin is None and other.value_origin is not None:
             self.value_origin = other.value_origin