Repository: EdinburghGenomics/pyclarity-lims

Issue:
all_inputs documentat - list vs dict_values
The documentation for "Process.all_inputs" infers that the output is always a list type variable but the output variable type can be list or dict_values

Process.all_inputs(self, unique=True, resolve=False) produces a list type variable
Process.all_inputs(self, uniqued=True, resolve=True) produces a dict_values

 def all_inputs(self, unique=True, resolve=False):
        """Retrieving all input artifacts from input_output_maps.
        If unique is true, no duplicates are returned.

        :param unique: boolean specifying if the list of artifacts should be uniqued
        :param resolve: boolean specifying if the artifacts entities should be resolved through a batch query.
        :return: list of input artifacts._

Discussion:

Patch:
diff --git a/docs/PracticalExamples.rst b/docs/PracticalExamples.rst
index 7c9a7e7..ee759a2 100644
--- a/docs/PracticalExamples.rst
+++ b/docs/PracticalExamples.rst
@@ -172,7 +172,7 @@ as the list of dictionary provided.
 .. code::
 
         # Assuming the Container c and the Project p exists.
-        l.create_batch(
+        samples = l.create_batch(
             Sample,
             [
                 {'container': c, 'project': p, 'name': 'sampletest1', 'position': 'H:1', 'udf':{'testudf': 'testudf_value1'}},
@@ -182,3 +182,16 @@ as the list of dictionary provided.
                 {'container': c, 'project': p, 'name': 'sampletest5', 'position': 'H:5', 'udf':{'testudf': 'testudf_value5'}}
             ]
         )
+
+.. warning::
+   The create_batch function returns entities already created with all attributes specified during the
+   creation populated. However it does not include attributes created on the LIMS side such as the artifact of samples.
+   These have to be retrieved manually using :py:func:`sample.get(force=True) <pyclarity_lims.entities.Entity.get>`
+   or :py:func:`lims.get_batch(samples, force=True) <pyclarity_lims.lims.Lims.get_batch>`
+
+.. code::
+
+        # After creation of the samples above
+        samples[0].artifact           # returns None
+        samples[0].get(force=True)    # retrieve the attribute as they are on the LIMS
+        samples[0].artifact           # returns Artifact(uri=...)
diff --git a/pyclarity_lims/lims.py b/pyclarity_lims/lims.py
index 3e1665c..14524a1 100644
--- a/pyclarity_lims/lims.py
+++ b/pyclarity_lims/lims.py
@@ -95,8 +95,17 @@ class Lims(object):
         else:
             return self.parse_response(r)
 
-    def get_file_contents(self, id=None, uri=None, encoding=None, crlf=False):
-        """Returns the contents of the file of <ID> or <uri>"""
+    def get_file_contents(self, id=None, uri=None, encoding=None, crlf=False, binary=False):
+        r"""
+        Download and returns the contents of the file of <ID> or <uri>.
+
+        :param id: the id of the file to retrieve.
+        :param uri: the uri of the file to retrieve.
+        :param encoding: When retrieve text file, this option can specify the encoding of the file.
+        :param crlf: When set to True the text file will be replace \\r\\n by \\n.
+        :param binary: When set to True the file content is returned as a binary stream.
+        :return: The file content in the format specify by the parameters.
+        """
         if id:
             url = self.get_uri('files', id, 'download')
         elif uri:
@@ -106,6 +115,8 @@ class Lims(object):
 
         r = self.request_session.get(url, auth=(self.username, self.password), timeout=TIMEOUT)
         self.validate_response(r)
+        if binary:
+            return r.content
         if encoding:
             r.encoding = encoding
 
@@ -637,7 +648,7 @@ class Lims(object):
             for node in root.getchildren():
                 instance = instance_map[node.attrib['limsid']]
                 instance.root = node
-        return instance_map.values()
+        return instances
 
     def put_batch(self, instances):
         """


Test patch:
diff --git a/tests/test_lims.py b/tests/test_lims.py
index f52619a..9e63470 100644
--- a/tests/test_lims.py
+++ b/tests/test_lims.py
@@ -3,7 +3,7 @@ from xml.etree import ElementTree
 
 from requests.exceptions import HTTPError
 
-from pyclarity_lims.entities import Sample, Project, Container
+from pyclarity_lims.entities import Sample, Project, Container, Artifact
 from pyclarity_lims.lims import Lims
 from tests import elements_equal
 
@@ -150,6 +150,9 @@ class TestLims(TestCase):
         assert lims.request_session.get.return_value.encoding == 'utf-16'
         lims.request_session.get.assert_called_with(exp_url, auth=(self.username, self.password), timeout=16)
 
+        lims.request_session = Mock(get=Mock(return_value=Mock(text='some data\n', content=b'some binary data')))
+        assert lims.get_file_contents(uri=self.url + '/api/v2/files/an_id', binary=True) == b'some binary data'
+
     def test_get_instances(self):
         lims = Lims(self.url, username=self.username, password=self.password)
         sample_xml_template = """<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
@@ -209,6 +212,32 @@ class TestLims(TestCase):
             assert len(samples) == 6
             assert mget.call_count == 3
 
+    def test_get_batch(self):
+        lims = Lims(self.url, username=self.username, password=self.password)
+        arts = [Artifact(lims, id='a1'), Artifact(lims, id='a2')]
+        artifact_list = '''<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+        <art:details xmlns:art="http://genologics.com/ri/artifact">
+          <art:artifact limsid="a1" uri="{url}/artifacts/a1">
+            <name>art1</name>
+            <type>type1</type>
+          </art:artifact>
+          <art:artifact limsid="a2" uri="{url}/artifacts/a2">
+            <name>art2</name>
+            <type>type2</type>
+          </art:artifact>
+        </art:details>
+        '''
+        with patch('requests.post', return_value=Mock(content=artifact_list, status_code=200)) as mocked_post:
+            arts = lims.get_batch(arts)
+            assert type(arts) == list
+            assert arts[0].name == 'art1'
+            assert arts[1].name == 'art2'
+            mocked_post.assert_called_once_with(
+                'http://testgenologics.com:4040/api/v2/artifacts/batch/retrieve',
+                auth=('test', 'password'),
+                data=b'<?xml version=\'1.0\' encoding=\'utf-8\'?>\n<ri:links xmlns:ri="http://genologics.com/ri"><link rel="artifacts" uri="http://testgenologics.com:4040/api/v2/artifacts/a1" /><link rel="artifacts" uri="http://testgenologics.com:4040/api/v2/artifacts/a2" /></ri:links>',
+                headers={'content-type': 'application/xml', 'accept': 'application/xml'}, params={})
+
     def test_create_batch(self):
         lims = Lims(self.url, username=self.username, password=self.password)
         p = Project(lims, uri='project')