Repository: SciLifeLab/genologics

Issue:
Change to StepActions
Hi,
I need to rework the way Step stores actions and that might results in backward incompatible changes to the API.
I was wondering how much this would affect you and the potential users of the API as it seems to be still work in progress
Cheers
Tim

Discussion:

Patch:
diff --git a/genologics/entities.py b/genologics/entities.py
index 22e0625..4b7bcf7 100644
--- a/genologics/entities.py
+++ b/genologics/entities.py
@@ -271,7 +271,6 @@ class BaseDescriptor(object):
     def __get__(self, instance, cls):
         raise NotImplementedError
 
-
 class TagDescriptor(BaseDescriptor):
     """Abstract base descriptor for an instance attribute
     represented by an XML element.
@@ -280,7 +279,6 @@ class TagDescriptor(BaseDescriptor):
     def __init__(self, tag):
         self.tag = tag
 
-
 class StringDescriptor(TagDescriptor):
     """An instance attribute containing a string value
     represented by an XML element.
@@ -307,7 +305,6 @@ class StringDescriptor(TagDescriptor):
         else:
             return instance.root
 
-
 class StringAttributeDescriptor(TagDescriptor):
     """An instance attribute containing a string value
     represented by an XML attribute.
@@ -317,7 +314,6 @@ class StringAttributeDescriptor(TagDescriptor):
         instance.get()
         return instance.root.attrib[self.tag]
 
-
 class StringListDescriptor(TagDescriptor):
     """An instance attribute containing a list of strings
     represented by multiple XML elements.
@@ -330,7 +326,6 @@ class StringListDescriptor(TagDescriptor):
             result.append(node.text)
         return result
 
-
 class StringDictionaryDescriptor(TagDescriptor):
     """An instance attribute containing a dictionary of string key/values
     represented by a hierarchical XML element.
@@ -345,7 +340,6 @@ class StringDictionaryDescriptor(TagDescriptor):
                 result[node2.tag] = node2.text
         return result
 
-
 class IntegerDescriptor(StringDescriptor):
     """An instance attribute containing an integer value
     represented by an XMl element.
@@ -359,7 +353,6 @@ class IntegerDescriptor(StringDescriptor):
         else:
             return int(node.text)
 
-
 class BooleanDescriptor(StringDescriptor):
     """An instance attribute containing a boolean value
     represented by an XMl element.
@@ -373,7 +366,6 @@ class BooleanDescriptor(StringDescriptor):
         else:
             return node.text.lower() == 'true'
 
-
 class UdfDictionary(object):
     "Dictionary-like container of UDFs, optionally within a UDT."
 
@@ -536,8 +528,6 @@ class UdfDictionary(object):
     def get(self, key, default=None):
         return self._lookup.get(key, default)
 
-
-
 class UdfDictionaryDescriptor(BaseDescriptor):
     """An instance attribute containing a dictionary of UDF values
     represented by multiple XML elements.
@@ -557,7 +547,6 @@ class UdtDictionaryDescriptor(UdfDictionaryDescriptor):
 
     _UDT = True
 
-
 class PlacementDictionaryDescriptor(TagDescriptor):
     """An instance attribute containing a dictionary of locations
     keys and artifact values represented by multiple XML elements.
@@ -571,7 +560,6 @@ class PlacementDictionaryDescriptor(TagDescriptor):
             self.value[key] = Artifact(instance.lims,uri=node.attrib['uri'])
         return self.value
 
-
 class ExternalidListDescriptor(BaseDescriptor):
     """An instance attribute yielding a list of tuples (id, uri) for
     external identifiers represented by multiple XML elements.
@@ -584,7 +572,6 @@ class ExternalidListDescriptor(BaseDescriptor):
             result.append((node.attrib.get('id'), node.attrib.get('uri')))
         return result
 
-
 class EntityDescriptor(TagDescriptor):
     "An instance attribute referencing another entity instance."
 
@@ -600,7 +587,6 @@ class EntityDescriptor(TagDescriptor):
         else:
             return self.klass(instance.lims, uri=node.attrib['uri'])
 
-
 class EntityListDescriptor(EntityDescriptor):
     """An instance attribute yielding a list of entity instances
     represented by multiple XML elements.
@@ -668,6 +654,7 @@ class NestedEntityListDescriptor(EntityListDescriptor):
         for node in rootnode.findall(self.tag):
             result.append(self.klass(instance.lims, uri=node.attrib['uri']))
         return result
+
 class DimensionDescriptor(TagDescriptor):
     """An instance attribute containing a dictionary specifying
     the properties of a dimension of a container type.
@@ -680,7 +667,6 @@ class DimensionDescriptor(TagDescriptor):
                     offset = int(node.find('offset').text),
                     size = int(node.find('size').text))
 
-
 class LocationDescriptor(TagDescriptor):
     """An instance attribute containing a tuple (container, value)
     specifying the location of an analyte in a container.
@@ -750,7 +736,6 @@ class Entity(object):
                 raise ValueError("Entity uri and id can't be both None")
             else:
                 uri = lims.get_uri(cls._URI, id)
-
         try:
             return lims.cache[uri]
         except KeyError:
@@ -950,6 +935,7 @@ class Process(Entity):
     udf            = UdfDictionaryDescriptor()
     udt            = UdtDictionaryDescriptor()
     files          = EntityListDescriptor(nsmap('file:file'), File)
+
     # instrument XXX
     # process_parameters XXX
 
@@ -1042,6 +1028,11 @@ class Process(Entity):
                 cs.append(o_a.container)
         return list(frozenset(cs))
 
+    @property
+    def step(self):
+        """Retrive the Step coresponding to this process. They share the same id"""
+        return Step(self.lims, id=self.id)
+
 class Artifact(Entity):
     "Any process input or output; analyte or file."
 
@@ -1105,28 +1096,76 @@ class Artifact(Entity):
     stateless = property(stateless) 
 
 class StepActions(Entity):
-    """Small hack to be able to query the actions subentity of
-    the Step entity. Right now, only the escalation is parsed."""
+    """Actions associated with a step"""
+    _escalation = None
 
-    def __init__(self, lims, uri=None, id=None):
-        super(StepActions, self).__init__(lims,uri,id)
-        self.escalation={}
-        self.lims=lims
-        self.root=self.lims.get(self.uri)
-        for node in self.root.findall('escalation'):
-            self.escalation['artifacts']=[]
-            self.escalation['author']=Researcher(lims,uri=node.find('request').find('author').attrib.get('uri'))
-            self.escalation['request']=uri=node.find('request').find('comment').text
-            if node.find('review') is not None: #recommended by the Etree doc
-                self.escalation['status']='Reviewed'
-                self.escalation['reviewer']= Researcher(lims,uri=node.find('review').find('author').attrib.get('uri'))
-                self.escalation['answer']=uri=node.find('review').find('comment').text
-            else:
-                self.escalation['status']='Pending'
+    @property
+    def escalation(self):
+        if not self._escalation:
+            self.get()
+            self._escalation={}
+            for node in self.root.findall('escalation'):
+                self._escalation['artifacts']=[]
+                self._escalation['author']=Researcher(self.lims,uri=node.find('request').find('author').attrib.get('uri'))
+                self._escalation['request']=uri=node.find('request').find('comment').text
+                if node.find('review') is not None: #recommended by the Etree doc
+                    self._escalation['status']='Reviewed'
+                    self._escalation['reviewer']= Researcher(self.lims,uri=node.find('review').find('author').attrib.get('uri'))
+                    self._escalation['answer']=uri=node.find('review').find('comment').text
+                else:
+                    self._escalation['status']='Pending'
+
+                for node2 in node.findall('escalated-artifacts'):
+                    art= self.lims.get_batch([Artifact(self.lims, uri=ch.attrib.get('uri')) for ch in node2])
+                    self._escalation['artifacts'].extend(art)
+        return self._escalation
+
+    @property
+    def next_actions(self):
+        actions = []
+        self.get()
+        if self.root.find('next-actions') is not None:
+            for node in self.root.find('next-actions').findall('next-action'):
+                action = {
+                    'artifact': Artifact(self.lims, node.attrib.get('artifact-uri')),
+                    'action': node.attrib.get('action'),
+                }
+                if node.attrib.get('step-uri'):
+                    action['step']=Step(self.lims, uri=node.attrib.get('step-uri'))
+                if node.attrib.get('rework-step-uri'):
+                    action['rework-step']=Step(self.lims, uri=node.attrib.get('rework-step-uri'))
+                actions.append(action)
+        return actions
+
+class ReagentKit(Entity):
+    """Type of Reagent with information about the provider"""
+    _URI="reagenttypes"
+    _TAG="reagent-kit"
+
+    name = StringDescriptor('name')
+    supplier = StringDescriptor('supplier')
+    website = StringDescriptor('website')
+    archived = BooleanDescriptor('archived')
 
-            for node2 in node.findall('escalated-artifacts'):
-                art= lims.get_batch([Artifact(lims,uri=ch.attrib.get('uri')) for ch in node2])
-                self.escalation['artifacts'].extend(art)
+class ReagentLot(Entity):
+    """Reagent Lots contain information about a particualr lot of reagent used in a step"""
+    _URI="reagentlot"
+    _TAG="reagent-lot"
+
+    reagent_kit = EntityDescriptor('reagent-kit', ReagentKit)
+    name = StringDescriptor('name')
+    lot_number = StringDescriptor('lot-number')
+    created_date = StringDescriptor('created-date')
+    last_modified_date = StringDescriptor('last-modified-date')
+    expiry_date = StringDescriptor('expiry-date')
+    created_by = EntityDescriptor('created-by', Researcher)
+    last_modified_by = EntityDescriptor('last-modified-by', Researcher)
+    status = StringDescriptor('status')
+    usage_count = IntegerDescriptor('usage-count')
+
+
+class StepReagentLots(Entity):
+    reagent_lots = NestedEntityListDescriptor('reagent-lot', ReagentLot, 'reagent-lots')
 
 
 class Step(Entity):
@@ -1134,16 +1173,15 @@ class Step(Entity):
 
     _URI = 'steps'
 
-    def __init__(self, lims, uri=None, id=None):
-        super(Step, self).__init__(lims,uri,id)
-        assert self.uri is not None
-        actionsuri="{0}/actions".format(self.uri)
-        self.actions= StepActions(lims,uri=actionsuri)
-
+    _reagent_lots       = EntityDescriptor('reagent-lots', StepReagentLots)
+    actions             = EntityDescriptor('actions', StepActions)
     #placements         = EntityDescriptor('placements', StepPlacements)
     #program_status     = EntityDescriptor('program-status',StepProgramStatus)
     #details            = EntityListDescriptor(nsmap('file:file'), StepDetails)
 
+    @property
+    def reagent_lots(self):
+        return self._reagent_lots.reagent_lots
 
 class ProtocolStep(Entity):
     """Steps key in the Protocol object"""
@@ -1173,6 +1211,7 @@ class Stage(Entity):
     """Holds Protocol/Workflow"""
     protocol = EntityDescriptor('protocol', Protocol)
 
+
 class Workflow(Entity):
     """ Workflow, introduced in 3.5"""
     _URI="configuration/workflows"
@@ -1200,10 +1239,10 @@ class ReagentType(Entity):
                     if child.attrib.get("name") == "Sequence":
                         self.sequence=child.attrib.get("value")
 
+
 Sample.artifact          = EntityDescriptor('artifact', Artifact)
 StepActions.step         = EntityDescriptor('step', Step)
 Stage.workflow            = EntityDescriptor('workflow', Workflow)
 Artifact.workflow_stages = NestedEntityListDescriptor('workflow-stage', Stage, 'workflow-stages')
 Step.configuration      = EntityDescriptor('configuration', ProtocolStep)
 
-
diff --git a/genologics/lims.py b/genologics/lims.py
index 3ee432a..316e9b1 100644
--- a/genologics/lims.py
+++ b/genologics/lims.py
@@ -489,9 +489,7 @@ class Lims(object):
         root = self.post(uri, data)
 
     def route_artifacts(self, artifact_list, workflow_uri=None, stage_uri=None, unassign=False):
-
         root = ElementTree.Element(nsmap('rt:routing'))
-
         if unassign:
             s = ElementTree.SubElement(root, 'unassign')
         else:
@@ -509,7 +507,6 @@ class Lims(object):
                           auth=(self.username, self.password),
                           headers={'content-type': 'application/xml',
                                    'accept': 'application/xml'})
-
         self.validate_response(r)
 
 
diff --git a/genologics/version.py b/genologics/version.py
index 18a95ec..b4cd250 100644
--- a/genologics/version.py
+++ b/genologics/version.py
@@ -1,1 +1,1 @@
-__version__="0.3.1"
+__version__="0.3.2"
diff --git a/setup.py b/setup.py
index 7aa61db..772f4e8 100644
--- a/setup.py
+++ b/setup.py
@@ -11,7 +11,8 @@ version_py = os.path.join(os.path.dirname(__file__), 'genologics', 'version.py')
 version = subprocess.Popen(["git", "describe", "--abbrev=0"],stdout=subprocess.PIPE, universal_newlines=True).communicate()[0].rstrip()
 if not version:
     version = __version__
-    
+else:
+    version = version.decode("utf-8")
 
 
 setup(name='genologics',


Test patch:
diff --git a/tests/test_entities.py b/tests/test_entities.py
index dbe16c6..2163bce 100644
--- a/tests/test_entities.py
+++ b/tests/test_entities.py
@@ -3,16 +3,18 @@ from xml.etree import ElementTree
 from sys import version_info
 from io import BytesIO
 
+from genologics.lims import Lims
+from genologics.entities import StringDescriptor, StringAttributeDescriptor, StringListDescriptor, \
+    StringDictionaryDescriptor, IntegerDescriptor, BooleanDescriptor, UdfDictionary, StepActions, Researcher, Artifact, \
+    Step
+
 if version_info.major == 2:
     from mock import patch, Mock
-    import __builtin__ as builtins
 else:
     from unittest.mock import patch, Mock
-    import builtins
 
 
-from genologics.entities import StringDescriptor, StringAttributeDescriptor, StringListDescriptor, \
-    StringDictionaryDescriptor, IntegerDescriptor, BooleanDescriptor, UdfDictionary
+
 
 
 class TestEntities(TestCase):
@@ -214,3 +216,86 @@ class TestUdfDictionary(TestCase):
     def test_get(self):
         pass
 
+
+
+class TestEntities(TestCase):
+    url = 'http://testgenologics.com:4040'
+    dummy_xml="""<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+    <dummy></dummy>"""
+
+    def setUp(self):
+        self.lims = Lims(self.url, username='test', password='password')
+
+class TestStepActions(TestEntities):
+    url = 'http://testgenologics.com:4040'
+    step_actions_xml = """<stp:actions xmlns:stp="http://genologics.com/ri/step" uri="...">
+  <step rel="..." uri="{url}/steps/s1">
+  </step>
+  <configuration uri="{url}/config/1">...</configuration>
+  <next-actions>
+    <next-action artifact-uri="{url}/artifacts/a1" action="requeue" step-uri="..." rework-step-uri="...">
+    </next-action>
+  </next-actions>
+  <escalation>
+    <request>
+      <author uri="{url}/researchers/r1">
+        <first-name>foo</first-name>
+        <last-name>bar</last-name>
+      </author>
+      <reviewer uri="{url}/researchers/r1">
+        <first-name>foo</first-name>
+        <last-name>bar</last-name>
+      </reviewer>
+      <date>01-01-1970</date>
+      <comment>no comments</comment>
+    </request>
+    <review>
+      <author uri="{url}/researchers/r1">
+        <first-name>foo</first-name>
+        <last-name>bar</last-name>
+      </author>
+      <date>01-01-1970</date>
+      <comment>no comments</comment>
+    </review>
+    <escalated-artifacts>
+      <escalated-artifact uri="{url}/artifacts/r1">
+      </escalated-artifact>
+    </escalated-artifacts>
+  </escalation>
+</stp:actions>""".format(url=url)
+
+    step_actions_no_escalation_xml = """<stp:actions xmlns:stp="http://genologics.com/ri/step" uri="...">
+  <step rel="..." uri="{url}/steps/s1">
+  </step>
+  <configuration uri="{url}/config/1">...</configuration>
+  <next-actions>
+    <next-action artifact-uri="{url}/artifacts/a1" action="requeue" step-uri="{url}/steps/s1" rework-step-uri="{url}/steps/s2">
+    </next-action>
+  </next-actions>
+</stp:actions>""".format(url=url)
+
+    def test_escalation(self):
+        s = StepActions(uri=self.lims.get_uri('steps', 'step_id', 'actions'), lims=self.lims)
+        with patch('requests.Session.get',return_value=Mock(content = self.step_actions_xml, status_code=200)),\
+             patch('requests.post', return_value=Mock(content = self.dummy_xml, status_code=200)):
+            r = Researcher(uri='http://testgenologics.com:4040/researchers/r1', lims=self.lims)
+            a = Artifact(uri='http://testgenologics.com:4040/artifacts/r1', lims=self.lims)
+            expected_escalation = {
+                'status': 'Reviewed',
+                'author': r,
+                'artifacts': [a], 'request': 'no comments',
+                'answer': 'no comments',
+                'reviewer': r}
+
+            assert s.escalation == expected_escalation
+
+    def test_next_actions(self):
+        s = StepActions(uri=self.lims.get_uri('steps', 'step_id', 'actions'), lims=self.lims)
+        with patch('requests.Session.get',return_value=Mock(content = self.step_actions_no_escalation_xml, status_code=200)):
+            step1 = Step(self.lims, uri='http://testgenologics.com:4040/steps/s1')
+            step2 = Step(self.lims, uri='http://testgenologics.com:4040/steps/s2')
+            artifact = Artifact(self.lims, uri='http://testgenologics.com:4040/artifacts/a1')
+            expected_next_actions = [{'artifact': artifact, 'action': 'requeue',
+                                      'step': step1, 'rework-step': step2}]
+            assert s.next_actions == expected_next_actions
+