diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e7f4ae4..845c9737 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,19 @@ XML files via lxml. See #301. The `odmlconversion` convenience console script has been added to convert multiple previous odML version files to the latest odML version. +## Changes in cloning behaviour + +When cloning a `Section` or a `Property` by default the id of any object is changed +to a new UUID. The cloning methods now feature a new `keep_id` attribute. If set to +`True`, the cloned object and any cloned children retain their original id. This +is meant to create exact copies of Section-Property trees in different documents. + +## Additional validation + +When a document is saved, a new validation check makes sure, that a document +contains only unique UUIDs this is required due to the introduction of creating +clones with identical ids. + # Version 1.4.1 diff --git a/odml/base.py b/odml/base.py index 979e54fb..1c64e5b0 100644 --- a/odml/base.py +++ b/odml/base.py @@ -581,7 +581,7 @@ def clean(self): for i in self: i.clean() - def clone(self, children=True): + def clone(self, children=True, keep_id=False): """ Clone this object recursively allowing to copy it independently to another document @@ -592,7 +592,7 @@ def clone(self, children=True): obj._sections = SmartList(BaseSection) if children: for s in self._sections: - obj.append(s.clone()) + obj.append(s.clone(keep_id=keep_id)) return obj diff --git a/odml/property.py b/odml/property.py index d3ea8b8d..92f92450 100644 --- a/odml/property.py +++ b/odml/property.py @@ -407,15 +407,20 @@ def get_path(self): return self.parent.get_path() + ":" + self.name - def clone(self): + def clone(self, keep_id=False): """ - Clone this object to copy it independently to another document. - The id of the cloned object will be set to a different uuid. + Clone this property to copy it independently to another document. + By default the id of the cloned object will be set to a different uuid. + + :param keep_id: If this attribute is set to True, the uuid of the + object will remain unchanged. + :return: The cloned property """ obj = super(BaseProperty, self).clone() obj._parent = None obj.value = self._value - obj._id = str(uuid.uuid4()) + if not keep_id: + obj.new_id() return obj diff --git a/odml/section.py b/odml/section.py index b026043f..3c476302 100644 --- a/odml/section.py +++ b/odml/section.py @@ -390,18 +390,26 @@ def remove(self, obj): else: raise ValueError("Can only remove sections and properties") - def clone(self, children=True): + def clone(self, children=True, keep_id=False): """ - Clone this object recursively allowing to copy it independently - to another document + Clone this Section allowing to copy it independently + to another document. By default the id of any cloned + object will be set to a new uuid. + + :param children: If True, also clone child sections and properties + recursively. + :param keep_id: If this attribute is set to True, the uuids of the + Section and all child objects will remain unchanged. + :return: The cloned Section. """ - obj = super(BaseSection, self).clone(children) - obj._id = str(uuid.uuid4()) + obj = super(BaseSection, self).clone(children, keep_id) + if not keep_id: + obj.new_id() obj._props = base.SmartList(BaseProperty) if children: for p in self._props: - obj.append(p.clone()) + obj.append(p.clone(keep_id)) return obj diff --git a/odml/tools/odmlparser.py b/odml/tools/odmlparser.py index 27fc9eb5..00dbdec0 100644 --- a/odml/tools/odmlparser.py +++ b/odml/tools/odmlparser.py @@ -50,7 +50,7 @@ def write_file(self, odml_document, filename): msg = "" for err in validation.errors: if err.is_error: - msg += "\n\t- %s %s: %s" % (err.obj, err.type, err.msg) + msg += "\n\t- %s %s: %s" % (err.obj, err.rank, err.msg) if msg != "": msg = "Resolve document validation errors before saving %s" % msg raise ParserException(msg) diff --git a/odml/validation.py b/odml/validation.py index 3113683f..0940f33e 100644 --- a/odml/validation.py +++ b/odml/validation.py @@ -174,7 +174,7 @@ def section_unique_ids(parent, id_map=None): yield i if sec.id in id_map: - yield ValidationError(sec, "Duplicate id in Section '%s' and '%s'" % + yield ValidationError(sec, "Duplicate id in Section '%s' and %s" % (sec.get_path(), id_map[sec.id])) else: id_map[sec.id] = "Section '%s'" % sec.get_path() @@ -203,7 +203,7 @@ def property_unique_ids(section, id_map=None): for prop in section.properties: if prop.id in id_map: - yield ValidationError(prop, "Duplicate id in Property '%s' and '%s'" % + yield ValidationError(prop, "Duplicate id in Property '%s' and %s" % (prop.get_path(), id_map[prop.id])) else: id_map[prop.id] = "Property '%s'" % prop.get_path() diff --git a/test/test_property.py b/test/test_property.py index 0877e3b3..e99a20da 100644 --- a/test/test_property.py +++ b/test/test_property.py @@ -623,6 +623,11 @@ def test_clone(self): self.assertIsNotNone(prop.parent) self.assertIsNone(clone_prop.parent) + # Check keep_id + prop = Property(name="keepid") + clone_prop = prop.clone(True) + self.assertEqual(prop.id, clone_prop.id) + def test_get_merged_equivalent(self): sec = Section(name="parent") mersec = Section(name="merged_section") diff --git a/test/test_section.py b/test/test_section.py index f4b82525..395a26d2 100644 --- a/test/test_section.py +++ b/test/test_section.py @@ -274,6 +274,30 @@ def test_clone(self): self.assertListEqual(clone_sec.sections, []) self.assertListEqual(clone_sec.properties, []) + def test_clone_keep_id(self): + # Check id kept in clone. + sec = Section(name="original") + clone_sec = sec.clone(keep_id=True) + self.assertEqual(sec, clone_sec) + self.assertEqual(sec.id, clone_sec.id) + + # Check cloned child Sections keep their ids. + Section(name="sec_one", parent=sec) + Section(name="sec_two", parent=sec) + clone_sec = sec.clone(keep_id=True) + self.assertListEqual(sec.sections, clone_sec.sections) + self.assertEqual(sec.sections["sec_one"], clone_sec.sections["sec_one"]) + self.assertEqual(sec.sections["sec_one"].id, clone_sec.sections["sec_one"].id) + + # Check cloned child Properties keep their ids. + Property(name="prop_one", parent=sec) + Property(name="prop_two", parent=sec) + clone_sec = sec.clone(keep_id=True) + self.assertListEqual(sec.properties, clone_sec.properties) + self.assertEqual(sec.properties["prop_one"], clone_sec.properties["prop_one"]) + self.assertEqual(sec.properties["prop_one"].id, + clone_sec.properties["prop_one"].id) + def test_reorder(self): # Test reorder of document sections doc = Document() diff --git a/test/test_validation.py b/test/test_validation.py index b2e5f7d4..df616fe0 100644 --- a/test/test_validation.py +++ b/test/test_validation.py @@ -98,3 +98,31 @@ def test_property_values(self): p.dependency = "p2" res = validate(doc) self.assertError(res, "non-existent dependency object") + + def test_property_unique_ids(self): + """ + Test if identical ids in properties raise a validation error + """ + doc = odml.Document() + sec_one = odml.Section("sec1", parent=doc) + sec_two = odml.Section("sec2", parent=doc) + prop = odml.Property("prop", parent=sec_one) + + cprop = prop.clone(keep_id=True) + sec_two.append(cprop) + + res = validate(doc) + self.assertError(res, "Duplicate id in Property") + + def test_section_unique_ids(self): + """ + Test if identical ids in sections raise a validation error. + """ + doc = odml.Document() + sec = odml.Section("sec", parent=doc) + + csec = sec.clone(keep_id=True) + sec.append(csec) + + res = validate(doc) + self.assertError(res, "Duplicate id in Section")