From 87f00e597e4a8933e3c70e14404f60b610af4012 Mon Sep 17 00:00:00 2001 From: "M. Sonntag" Date: Thu, 25 Oct 2018 16:15:54 +0200 Subject: [PATCH 01/11] [tools/odmlparser] Fix validation attribute rename odml.Validation attribute 'type' was renamed to 'rank'. --- odml/tools/odmlparser.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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) From 079f6d643eb1661b6bf9e0ecc76d6f482dabdd9a Mon Sep 17 00:00:00 2001 From: "M. Sonntag" Date: Thu, 25 Oct 2018 16:41:58 +0200 Subject: [PATCH 02/11] [property] Add 'keep_id' flag to 'clone' Give the option to keep the uuid when a property is cloned; default is to create a new uuid for a cloned property. --- odml/property.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/odml/property.py b/odml/property.py index d3ea8b8d..1d2f604b 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._id = str(uuid.uuid4()) return obj From 460683760b18506ab78c3d50a9ddf9bb419518ee Mon Sep 17 00:00:00 2001 From: "M. Sonntag" Date: Thu, 25 Oct 2018 16:43:57 +0200 Subject: [PATCH 03/11] [test/property] Add clone keep_id test --- test/test_property.py | 5 +++++ 1 file changed, 5 insertions(+) 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") From 6dace91f32fa3d70f942572936de28d022117763 Mon Sep 17 00:00:00 2001 From: "M. Sonntag" Date: Thu, 25 Oct 2018 16:44:24 +0200 Subject: [PATCH 04/11] [test/validation] Add unique property id test --- test/test_validation.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/test_validation.py b/test/test_validation.py index b2e5f7d4..55ea6b2f 100644 --- a/test/test_validation.py +++ b/test/test_validation.py @@ -98,3 +98,18 @@ 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") From 1471752ecbcfd5f43f6b3d9bbeb9d7504d8daf61 Mon Sep 17 00:00:00 2001 From: "M. Sonntag" Date: Thu, 25 Oct 2018 16:48:35 +0200 Subject: [PATCH 05/11] [property] Use 'new_id' method when cloning --- odml/property.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/odml/property.py b/odml/property.py index 1d2f604b..92f92450 100644 --- a/odml/property.py +++ b/odml/property.py @@ -420,7 +420,7 @@ def clone(self, keep_id=False): obj._parent = None obj.value = self._value if not keep_id: - obj._id = str(uuid.uuid4()) + obj.new_id() return obj From 7d6944208cf5f756b52062f2242f393da5edcec4 Mon Sep 17 00:00:00 2001 From: "M. Sonntag" Date: Thu, 25 Oct 2018 16:54:08 +0200 Subject: [PATCH 06/11] [section] Use 'new_id' method when cloning --- odml/section.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/odml/section.py b/odml/section.py index b026043f..485ea7c5 100644 --- a/odml/section.py +++ b/odml/section.py @@ -396,7 +396,7 @@ def clone(self, children=True): to another document """ obj = super(BaseSection, self).clone(children) - obj._id = str(uuid.uuid4()) + obj.new_id() obj._props = base.SmartList(BaseProperty) if children: From 8fae3e3acfe460617f6c9712cd36ebbbb3ab2fbd Mon Sep 17 00:00:00 2001 From: "M. Sonntag" Date: Thu, 25 Oct 2018 17:31:13 +0200 Subject: [PATCH 07/11] [base/section] Add 'keep_id' flag to 'clone' Closes #259, closes #258 --- odml/base.py | 4 ++-- odml/section.py | 20 ++++++++++++++------ 2 files changed, 16 insertions(+), 8 deletions(-) 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/section.py b/odml/section.py index 485ea7c5..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.new_id() + 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 From 9e70b8d7d2ea47a7832887bdaee5b5245a5e0bbc Mon Sep 17 00:00:00 2001 From: "M. Sonntag" Date: Thu, 25 Oct 2018 17:34:09 +0200 Subject: [PATCH 08/11] [test/section] Add clone keep_id test --- test/test_section.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) 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() From 3e74d4812c5ed5e9c774dfb005216ee961f6aba8 Mon Sep 17 00:00:00 2001 From: "M. Sonntag" Date: Thu, 25 Oct 2018 17:38:45 +0200 Subject: [PATCH 09/11] [test/validation] Add unique section id test --- test/test_validation.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/test/test_validation.py b/test/test_validation.py index 55ea6b2f..df616fe0 100644 --- a/test/test_validation.py +++ b/test/test_validation.py @@ -113,3 +113,16 @@ def test_property_unique_ids(self): 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") From f2b9f88e11aae62e494b0a6f5f45d81391b95c0a Mon Sep 17 00:00:00 2001 From: "M. Sonntag" Date: Thu, 25 Oct 2018 17:39:08 +0200 Subject: [PATCH 10/11] [validation] Cleanup duplicate id messages --- odml/validation.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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() From f99896f9b674fa0e44d9722fe41a16b81aef8675 Mon Sep 17 00:00:00 2001 From: "M. Sonntag" Date: Thu, 25 Oct 2018 17:52:40 +0200 Subject: [PATCH 11/11] [CHANGELOG] Add cloning behaviour changes --- CHANGELOG.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) 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