Repository: EdinburghGenomics/pyclarity-lims

Issue:
UdfConfig presets clear method clears the entire UdfConfig object
Calling the UdfConfig.preset.clear() method seems to clear values from the entire UdfConfig object. Even if I set the preset list manually to a new empty list the object is also cleared...Is this expected?

Discussion:

Patch:
diff --git a/pyclarity_lims/descriptors.py b/pyclarity_lims/descriptors.py
index c2ce1ad..1c4eea6 100644
--- a/pyclarity_lims/descriptors.py
+++ b/pyclarity_lims/descriptors.py
@@ -78,7 +78,8 @@ class XmlDictionary(XmlMutable, dict):
 
     def clear(self):
         dict.clear(self)
-        self.rootnode(self.instance).clear()
+        for elem in self._elems:
+            self.rootnode(self.instance).remove(elem)
         self._update_elems()
 
     def _update_elems(self):
@@ -337,18 +338,21 @@ class PlacementDictionary(XmlDictionary):
                 break
 
 
-class SubTagDictionary(XmlDictionary):
+class SubTagDictionary(XmlDictionary, Nestable):
     """Dictionary of xml sub element where the key is the tag
     and the value is the text of the sub element.
     """
     def __init__(self, instance, tag, **kwargs):
-        self.tag = tag
+        # In case extra nesting is provided
+        nesting = kwargs.get('nesting', [])
+        nesting.append(tag)
+        Nestable.__init__(self, nesting=nesting)
         XmlDictionary.__init__(self, instance)
 
     def _update_elems(self):
-        tag_node = self.rootnode(self.instance).find(self.tag)
-        if tag_node:
-            self._elems = tag_node.getchildren()
+        root_node = self.rootnode(self.instance)
+        if root_node:
+            self._elems = root_node.getchildren()
         else:
             self._elems = []
 
@@ -358,20 +362,18 @@ class SubTagDictionary(XmlDictionary):
     def _setitem(self, key, value):
         if not isinstance(key, str):
             raise ValueError()
-        tag_node = self.rootnode(self.instance).find(self.tag)
-        if tag_node is None:
-            tag_node = ElementTree.SubElement(self.rootnode(self.instance), self.tag)
+        root_node = self.rootnode(self.instance)
 
-        elem = tag_node.find(key)
+        elem = root_node.find(key)
         if elem is None:
-            elem = ElementTree.SubElement(tag_node, key)
+            elem = ElementTree.SubElement(root_node, key)
         elem.text = value
 
     def _delitem(self, key):
-        tag_node = self.rootnode(self.instance).find(self.tag)
+        root_node = self.rootnode(self.instance)
         for node in self._elems:
             if node.tag == key:
-                tag_node.remove(node)
+                root_node.remove(node)
                 break
 
 
@@ -437,7 +439,8 @@ class XmlList(XmlMutable, list):
     def clear(self):
         # python 2.7 does not have a clear function for list
         del self[:]
-        self.rootnode(self.instance).clear()
+        for elem in self._elems:
+            self.rootnode(self.instance).remove(elem)
         self._update_elems()
 
     def __add__(self, other_list):


Test patch:
diff --git a/tests/test_descriptors.py b/tests/test_descriptors.py
index e4cee66..bd8a42f 100644
--- a/tests/test_descriptors.py
+++ b/tests/test_descriptors.py
@@ -191,6 +191,8 @@ class TestStringAttributeDescriptor(TestDescriptor):
         assert instance_new.root.attrib['name'] == "test name2"
 
 
+
+
 class TestStringListDescriptor(TestDescriptor):
     def setUp(self):
         et = ElementTree.fromstring("""<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
@@ -368,7 +370,7 @@ class TestUdfDictionary(TestCase):
             assert (k, self.dict1[k]) in expected_content
 
 
-class TestPlacementDictionary(TestCase):
+class TestPlacementDictionary(TestDescriptor):
 
     def setUp(self):
         et = ElementTree.fromstring("""<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
@@ -376,6 +378,7 @@ class TestPlacementDictionary(TestCase):
 <placement uri="http://testgenologics.com:4040/api/v2/artifacts/a1" limsid="a1">
 <value>A:1</value>
 </placement>
+<other>thing</other>
 </test-entry>""")
         self.lims = Lims('http://testgenologics.com:4040', username='test', password='password')
         self.instance1 = Mock(root=et, lims=self.lims)
@@ -410,6 +413,15 @@ class TestPlacementDictionary(TestCase):
         del self.dict1['A:1']
         assert len(self.dict1.rootnode(self.dict1.instance).findall('placement')) == 0
 
+    def test_clear(self):
+        el = EntityList(self.instance1, 'artifact', Artifact)
+        sd = self._make_desc(StringDescriptor, 'other')
+        assert sd.__get__(self.instance1, None) == "thing"
+        assert len(self.dict1.rootnode(self.dict1.instance).findall('placement')) == 1
+        self.dict1.clear()
+        assert len(self.dict1.rootnode(self.dict1.instance).findall('placement')) == 0
+        assert sd.__get__(self.instance1, None) == "thing"
+
 
 class TestSubTagDictionary(TestCase):
 
@@ -424,20 +436,32 @@ class TestSubTagDictionary(TestCase):
         self.instance1 = Mock(root=et, lims=self.lims)
         self.dict1 = SubTagDictionary(self.instance1, tag='test-tag')
 
+        et = ElementTree.fromstring("""<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+        <test-entry xmlns:udf="http://genologics.com/ri/userdefined">
+        </test-entry>""")
+        self.instance2 = Mock(root=et, lims=self.lims)
+        self.dict2 = SubTagDictionary(self.instance2, tag='test-tag')
+
     def test___getitem__(self):
         assert self.dict1['key1'] == 'value1'
 
     def test___setitem__(self):
-        assert len(self.dict1.rootnode(self.dict1.instance).find('test-tag')) == 1
-        assert self.dict1.rootnode(self.dict1.instance).find('test-tag').find('key1').text == 'value1'
+        assert len(self.dict1.rootnode(self.dict1.instance)) == 1
+        assert self.dict1.rootnode(self.dict1.instance).find('key1').text == 'value1'
         self.dict1['key1'] = 'value11'
-        assert len(self.dict1.rootnode(self.dict1.instance).find('test-tag')) == 1
-        assert self.dict1.rootnode(self.dict1.instance).find('test-tag').find('key1').text == 'value11'
+        assert len(self.dict1.rootnode(self.dict1.instance)) == 1
+        assert self.dict1.rootnode(self.dict1.instance).find('key1').text == 'value11'
         self.dict1['key2'] = 'value2'
-        assert len(self.dict1.rootnode(self.dict1.instance).find('test-tag')) == 2
-        assert self.dict1.rootnode(self.dict1.instance).find('test-tag').find('key2').text == 'value2'
+        assert len(self.dict1.rootnode(self.dict1.instance)) == 2
+        assert self.dict1.rootnode(self.dict1.instance).find('key2').text == 'value2'
         assert self.dict1['key2'] == 'value2'
 
+    def test___setitem__from_empty(self):
+        assert len(self.dict2.rootnode(self.dict2.instance)) == 0
+        self.dict2['key1'] = 'value1'
+        assert self.dict2.rootnode(self.dict2.instance).find('key1').text == 'value1'
+        assert len(self.dict2.rootnode(self.dict2.instance)) == 1
+
 
 class TestXmlElementAttributeDict(TestCase):
     def setUp(self):
@@ -502,13 +526,14 @@ class TestXmlPooledInputDict(TestCase):
         assert len(self.dict1.rootnode(self.dict1.instance)) == 3
 
 
-class TestEntityList(TestCase):
+class TestEntityList(TestDescriptor):
 
     def setUp(self):
         et = ElementTree.fromstring("""<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
     <test-entry>
     <artifact uri="http://testgenologics.com:4040/api/v2/artifacts/a1"></artifact>
     <artifact uri="http://testgenologics.com:4040/api/v2/artifacts/a2"></artifact>
+    <other>thing</other>
     </test-entry>
     """)
         self.lims = Lims('http://testgenologics.com:4040', username='test', password='password')
@@ -579,6 +604,15 @@ class TestEntityList(TestCase):
         assert el[0] == a3
         assert el[1] == a4
 
+    def test_clear(self):
+        el = EntityList(self.instance1, 'artifact', Artifact)
+        sd = self._make_desc(StringDescriptor, 'other')
+        assert sd.__get__(self.instance1, None) == "thing"
+        assert len(el) == 2
+        el.clear()
+        assert len(el) == 0
+        assert sd.__get__(self.instance1, None) == "thing"
+
 
 class TestInputOutputMapList(TestCase):
     def setUp(self):
@@ -751,7 +785,7 @@ class TestQueuedArtifactList(TestCase):
                 </location>
                 </artifact>
                 <artifact uri="{url}/artifacts/a3">
-                <queue-time>2011-12-25T01:10:10.050-00:00</queue-time>
+                <queue-time>2011-12-25T01:10:10.050-01:00</queue-time>
                 <location>
                 <container uri="{url}/containers/c1"/>
                 <value>A:3</value>
@@ -783,7 +817,7 @@ class TestQueuedArtifactList(TestCase):
         assert queued_artifacts[0] == qart
         qart = self.get_queue_art('a2', 'A:2', 200000, datetime.timedelta(0, 3600))
         assert queued_artifacts[1] == qart
-        qart = self.get_queue_art('a3', 'A:3', 50000, datetime.timedelta(0, 0))
+        qart = self.get_queue_art('a3', 'A:3', 50000, datetime.timedelta(0, -3600))
         assert queued_artifacts[2] == qart
 
     def test_set(self):