Repository: EdinburghGenomics/pyclarity-lims

Issue:
Lims.get_reagent_types is defined twice
Which one do we want?

Discussion:

Patch:
diff --git a/pyclarity_lims/entities.py b/pyclarity_lims/entities.py
index 9d22d8d..80bfaa1 100644
--- a/pyclarity_lims/entities.py
+++ b/pyclarity_lims/entities.py
@@ -200,6 +200,9 @@ class Note(Entity):
 class File(Entity):
     """File attached to a project or a sample."""
 
+    _URI = 'files'
+    _PREFIX = 'file'
+
     attached_to = StringDescriptor('attached-to')
     """The uri of the Entity this file is attached to"""
     content_location = StringDescriptor('content-location')
@@ -379,7 +382,7 @@ class Process(Entity):
     _CREATION_PREFIX = 'prx'
 
     type = EntityDescriptor('type', Processtype)
-    """The :py:class:`type <pyclarity_lims.entities.ProcessType>` of the process"""
+    """The :py:class:`type <pyclarity_lims.entities.Processtype>` of the process"""
     date_run = StringDescriptor('date-run')
     """The date at which the process was finished in format Year-Month-Day i.e. 2016-12-05."""
     technician = EntityDescriptor('technician', Researcher)
@@ -407,7 +410,10 @@ class Process(Entity):
 
     """
     udf = UdfDictionaryDescriptor()
-    """Dictionary of UDFs associated with the process."""
+    """Dictionary of UDFs associated with the process. 
+    
+    Note that the UDFs cannot be modify in Process. Use :py:class:`Step details <pyclarity_lims.entities.StepDetails>`
+    to modify UDFs instead. You can access them with process.step.details.udf"""
     udt = UdtDictionaryDescriptor()
     """Dictionary of UDTs associated with the process."""
     files = EntityListDescriptor(nsmap('file:file'), File)
@@ -420,13 +426,17 @@ class Process(Entity):
     def outputs_per_input(self, inart, ResultFile=False, SharedResultFile=False, Analyte=False):
         """Getting all the output artifacts related to a particular input artifact
 
-        :param inart: input artifact id use to select the output
+        :param inart: input artifact id or artifact entity use to select the output
         :param ResultFile: boolean specifying to only return ResultFiles.
         :param SharedResultFile: boolean specifying to only return SharedResultFiles.
         :param Analyte: boolean specifying to only return Analytes.
         :return: output artifact corresponding to the input artifact provided
         """
-        inouts = [io for io in self.input_output_maps if io[0]['limsid'] == inart]
+        if isinstance(Artifact, inart):
+            inouts = [io for io in self.input_output_maps if io[0]['uri'] == inart]
+        else:
+            inouts = [io for io in self.input_output_maps if io[0]['limsid'] == inart]
+
         if ResultFile:
             inouts = [io for io in inouts if io[1]['output-type'] == 'ResultFile']
         elif SharedResultFile:
@@ -490,15 +500,38 @@ class Process(Entity):
         else:
             return [Artifact(self.lims, id=id) for id in ids if id is not None]
 
-    def shared_result_files(self):
-        """Retrieve all resultfiles of output-generation-type PerAllInputs."""
-        artifacts = self.all_outputs(unique=True)
-        return [a for a in artifacts if a.output_type == 'SharedResultFile']
+    def _output_files(self, resfile, output_generation_type):
+        if output_generation_type:
+            artifacts = [
+                io[1]['uri'] for io in self.input_output_maps
+                if io[1] is not None
+                and io[1]['output-type'] == resfile
+                and io[1]['output-generation-type'] == output_generation_type
+            ]
+        else:
+            artifacts = [
+                io[1]['uri'] for io in self.input_output_maps
+                if io[1] is not None and io[1]['output-type'] == resfile
+            ]
+        return list(set(artifacts))
 
-    def result_files(self):
-        """Retrieve all resultfiles of output-generation-type perInput."""
-        artifacts = self.all_outputs(unique=True)
-        return [a for a in artifacts if a.output_type == 'ResultFile']
+    def shared_result_files(self, output_generation_type=None):
+        """Retrieve all output artifacts where output-type is SharedResultFile.
+
+        :param output_generation_type: string specifying the output-generation-type (PerAllInputs or PerInput)
+        :return: list of output artifacts.
+
+        """
+        return self._output_files('SharedResultFile', output_generation_type)
+
+    def result_files(self, output_generation_type=None):
+        """Retrieve all output artifacts where output-type is ResultFile.
+
+        :param output_generation_type: string specifying the output-generation-type (PerAllInputs or PerInput)
+        :return: list of output artifacts.
+
+        """
+        return self._output_files('ResultFile', output_generation_type)
 
     def analytes(self):
         """Retrieving the output Analytes of the process, if existing.
diff --git a/pyclarity_lims/lims.py b/pyclarity_lims/lims.py
index 532b315..ee33165 100644
--- a/pyclarity_lims/lims.py
+++ b/pyclarity_lims/lims.py
@@ -231,7 +231,7 @@ class Lims(object):
                                   start_index=start_index)
         return self._get_instances(Udfconfig, add_info=add_info, nb_pages=nb_pages, params=params)
 
-    def get_reagent_types(self, name=None, start_index=None, nb_pages=-1):
+    def get_reagent_types(self, name=None, start_index=None, nb_pages=-1, add_info=False):
         """
         Get a list of reagent types, filtered by keyword arguments.
 
@@ -239,10 +239,12 @@ class Lims(object):
         :param start_index: first element to retrieve; start at first element if None.
         :param nb_pages: number of page to iterate over. The page size is 500 by default unless configured otherwise
                         in your LIMS. 0 or negative numbers returns all pages.
+        :param add_info: Change the return type to a tuple where the first element is normal return and
+                        the second is a dict of additional information provided in the query.
+
         """
-        params = self._get_params(name=name,
-                                  start_index=start_index)
-        return self._get_instances(ReagentType, nb_pages=nb_pages, params=params)
+        params = self._get_params(name=name, start_index=start_index)
+        return self._get_instances(ReagentType, nb_pages=nb_pages, add_info=add_info, params=params)
 
     def get_labs(self, name=None, last_modified=None,
                  udf=dict(), udtname=None, udt=dict(), start_index=None, nb_pages=-1, add_info=False):
@@ -508,18 +510,6 @@ class Lims(object):
         params = self._get_params(displayname=displayname)
         return self._get_instances(Processtype, add_info=add_info, params=params)
 
-    def get_reagent_types(self, name=None, add_info=False):
-        """
-       Get a list of reagent types with the specified name.
-
-       :param name: The name the reagent type
-       :param add_info: Change the return type to a tuple where the first element is normal return and
-                        the second is a dict of additional information provided in the query.
-
-       """
-        params = self._get_params(name=name)
-        return self._get_instances(ReagentType, add_info=add_info, params=params)
-
     def get_protocols(self, name=None, add_info=False):
         """
         Get a list of existing protocols on the system.


Test patch:
diff --git a/tests/test_entities.py b/tests/test_entities.py
index f7835fc..27f5e9b 100644
--- a/tests/test_entities.py
+++ b/tests/test_entities.py
@@ -2,7 +2,7 @@ from sys import version_info
 from unittest import TestCase
 from xml.etree import ElementTree
 from pyclarity_lims.entities import ProtocolStep, StepActions, Researcher, Artifact, \
-    Step, StepPlacements, Container, Stage, ReagentKit, ReagentLot, Sample, Project
+    Step, StepPlacements, Container, Stage, ReagentKit, ReagentLot, Sample, Project, Process
 from pyclarity_lims.lims import Lims
 from tests import NamedMock, elements_equal
 if version_info[0] == 2:
@@ -163,6 +163,43 @@ generic_sample_creation_xml = """
 </smp:samplecreation>
 """
 
+generic_process_xml = """
+<prc:process xmlns:udf="http://genologics.com/ri/userdefined" xmlns:file="http://genologics.com/ri/file" xmlns:prc="http://genologics.com/ri/process" uri="{url}/api/v2/processes/p2" limsid="p2">
+<type uri="{url}/api/v2/processtypes/pt1">Step Name 5.0</type>
+<date-run>2018-06-12</date-run>
+<technician uri="{url}/api/v2/researchers/1">
+<first-name>Bob</first-name>
+<last-name>Marley</last-name>
+</technician>
+<input-output-map>
+<input post-process-uri="{url}/api/v2/artifacts/a1" uri="{url}/api/v2/artifacts/a1" limsid="a1">
+<parent-process uri="{url}/api/v2/processes/p1" limsid="p1"/>
+</input>
+<output uri="{url}/api/v2/artifacts/ao1" output-generation-type="PerAllInputs" output-type="ResultFile" limsid="ao1"/>
+</input-output-map>
+<input-output-map>
+<input post-process-uri="{url}/api/v2/artifacts/a2" uri="{url}/api/v2/artifacts/a2" limsid="a2">
+<parent-process uri="{url}/api/v2/processes/p1" limsid="p1"/>
+</input>
+<output uri="{url}/api/v2/artifacts/ao1" output-generation-type="PerAllInputs" output-type="ResultFile" limsid="ao1"/>
+</input-output-map>
+<input-output-map>
+<input post-process-uri="{url}/api/v2/artifacts/a1" uri="{url}/api/v2/artifacts/a1" limsid="a1">
+<parent-process uri="{url}/api/v2/processes/p1" limsid="p1"/>
+</input>
+<output uri="{url}/api/v2/artifacts/ao2" output-generation-type="PerInput" output-type="ResultFile" limsid="ao2"/>
+</input-output-map>
+<input-output-map>
+<input post-process-uri="{url}/api/v2/artifacts/a2" uri="{url}/api/v2/artifacts/a2" limsid="a2">
+<parent-process uri="{url}/api/v2/processes/p1" limsid="p1"/>
+</input>
+<output uri="{url}/api/v2/artifacts/ao3" output-generation-type="PerInput" output-type="ResultFile" limsid="ao3"/>
+</input-output-map>
+<udf:field type="Numeric" name="Count">15</udf:field>
+<process-parameter name="parameter1"/>
+</prc:process>
+"""
+
 
 class TestEntities(TestCase):
     dummy_xml = """<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
@@ -430,3 +467,14 @@ class TestSample(TestEntities):
             </location>
             </smp:samplecreation>'''
             assert elements_equal(ElementTree.fromstring(patch_post.call_args_list[0][1]['data']), ElementTree.fromstring(data))
+
+
+class TestProcess(TestEntities):
+    process_xml = generic_process_xml.format(url=url)
+
+    def test_result_files(self):
+        p = Process(uri=self.lims.get_uri('processes', 'p2'), lims=self.lims)
+        with patch('requests.Session.get', return_value=Mock(content=self.process_xml, status_code=200)):
+            assert len(p.result_files()) == 3
+            assert len(p.result_files(output_generation_type='PerAllInputs')) == 1
+            assert len(p.result_files(output_generation_type='PerInput')) == 2