Repository: phac-nml/irida-uploader

Issue:
Full file path support for directory uploader
## Describe your idea for a new feature
Currently the directory uploader only supports file names listed in specified directory, like so
```
[Data]
Sample_Name,Project_ID,File_Forward,File_Reverse
my-sample-1,5,file_1.fastq.gz,
my-sample-2,5,samp_F.fastq.gz,
```
It should be able to use the following SampleLIst.csv file
```
[Data]
Sample_Name,Project_ID,File_Forward,File_Reverse
my-sample-1,5,/home/user/example_sample_sheets/directory_run/file_1.fastq.gz,
my-sample-2,5,/home/user/example_sample_sheets/directory_run/samp_F.fastq.gz,
```

## Additional information
This would be very useful for scripting, mainly because we could generate samplesheets on a different network location than the files themselves are.


Discussion:

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index c2b20ad..682c1eb 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -3,6 +3,9 @@ Changes
 
 Beta 0.4.3
 ----------
+Added functionality:
+* `directory` parsers now support full file paths for upload.
+
 Developer changes:
 * Added method `get_metadata(self, sample_name, project_id)` to allow other metadata to be fetched from the `api` module
 * Added method `send_metadata(self, metadata, project_id, sample_name)` to allow other metadata to be sent using the `api` module
diff --git a/docs/parsers/directory.md b/docs/parsers/directory.md
index 8fe0af4..0bb3c1e 100644
--- a/docs/parsers/directory.md
+++ b/docs/parsers/directory.md
@@ -55,3 +55,11 @@ my-sample-1,75,file_1.fastq.gz
 my-sample-2,75,samp_F.fastq.gz
 my-sample-3,76,germ_f.fastq.gz
 ```
+
+Alternatively, full file paths can be used when specifying files:
+```
+[Data]
+Sample_Name,Project_ID,File_Forward,File_Reverse
+my-sample-1,5,/home/user/my_file_dir/directory_run/file_1.fastq.gz,
+my-sample-2,5,/home/user/my_file_dir/directory_run/samp_F.fastq.gz,
+```
diff --git a/iridauploader/core/cli_entry.py b/iridauploader/core/cli_entry.py
index 8f15c79..c0cb880 100644
--- a/iridauploader/core/cli_entry.py
+++ b/iridauploader/core/cli_entry.py
@@ -9,7 +9,7 @@ from iridauploader.model import DirectoryStatus
 
 from . import api_handler, parsing_handler, logger, exit_return
 
-VERSION_NUMBER = "0.4.2"
+VERSION_NUMBER = "0.4.3"
 
 
 def upload_run_single_entry(directory, force_upload=False, upload_assemblies=False):
diff --git a/iridauploader/parsers/directory/parser.py b/iridauploader/parsers/directory/parser.py
index a64fd2d..c70a4f6 100644
--- a/iridauploader/parsers/directory/parser.py
+++ b/iridauploader/parsers/directory/parser.py
@@ -2,7 +2,6 @@ import logging
 import os
 
 import iridauploader.progress as progress
-import iridauploader.model as model
 
 from iridauploader.parsers import exceptions
 from iridauploader.parsers import common
@@ -87,30 +86,52 @@ class Parser:
         Throws a ValidationError with a validation result attached if it cannot make a sequencing run
 
         :param sample_sheet:
+        :param run_data_directory_file_list: Optional: List of files in the data directory to verify against the
+        SampleList.csv file. This is used when deploying the parsers on a cloud environment.
         :return: SequencingRun
         """
 
-        # get file list
-        validation_result = model.ValidationResult()
+        # Try to get the sample sheet, validate that the sample sheet is valid
+        validation_result = validation.validate_sample_sheet(sample_sheet)
+        if not validation_result.is_valid():
+            logging.error("Errors occurred while getting sample sheet")
+            raise exceptions.ValidationError("Errors occurred while getting sample sheet", validation_result)
 
+        # When running with a premade file list, verify files on sample_sheet are in file list
         try:
-            if run_data_directory_file_list is None:
-                data_dir = os.path.dirname(sample_sheet)
-                run_data_directory_file_list = common.get_file_list(data_dir)
+            if run_data_directory_file_list is not None:
+                sample_parser.verify_sample_sheet_file_names_in_file_list(sample_sheet, run_data_directory_file_list)
+        except exceptions.SequenceFileError as error:
+            validation_result.add_error(error)
+            logging.error("Errors occurred while building sequence run from sample sheet")
+            raise exceptions.ValidationError("Errors occurred while building sequence run from sample sheet",
+                                             validation_result)
+
+        # Build a list of sample objects from sample sheet
+        try:
+            if run_data_directory_file_list is not None:
+                sample_list = sample_parser.build_sample_list_from_sample_sheet_no_verify(sample_sheet)
+            else:
+                sample_list = sample_parser.build_sample_list_from_sample_sheet_with_abs_path(sample_sheet)
         except exceptions.DirectoryError as error:
             validation_result.add_error(error)
             logging.error("Errors occurred while parsing files")
             raise exceptions.ValidationError("Errors occurred while parsing files", validation_result)
 
-        # Try to get the sample sheet, validate that the sample sheet is valid
-        validation_result = validation.validate_sample_sheet(sample_sheet)
-        if not validation_result.is_valid():
-            logging.error("Errors occurred while getting sample sheet")
-            raise exceptions.ValidationError("Errors occurred while getting sample sheet", validation_result)
+        # verify samples in sample_list are all of one type, either single or paired end
+        if not sample_parser.only_single_or_paired_in_sample_list(sample_list):
+            e = exceptions.SampleSheetError(
+                ("Your sample sheet is malformed. "
+                 "SampleSheet cannot have both paired end and single end runs. "
+                 "Make sure all samples are either paired or single."),
+                sample_sheet
+            )
+            validation_result.add_error(e)
+            logging.error("Error occurred while building file list: Sample sheet has both paired and single end reads")
+            raise exceptions.ValidationError("Errors occurred while building file list.", validation_result)
 
         # Try to build sequencing run from sample sheet & meta data, raise validation error if errors occur
         try:
-            sample_list = sample_parser.parse_sample_list(sample_sheet, run_data_directory_file_list)
             run_metadata = sample_parser.parse_metadata(sample_list)
             sequencing_run = common.build_sequencing_run_from_samples(sample_list, run_metadata)
         except exceptions.SequenceFileError as error:
diff --git a/iridauploader/parsers/directory/sample_parser.py b/iridauploader/parsers/directory/sample_parser.py
index 7cd388a..d8b373d 100644
--- a/iridauploader/parsers/directory/sample_parser.py
+++ b/iridauploader/parsers/directory/sample_parser.py
@@ -1,4 +1,4 @@
-from os import path, walk
+from os import path
 from collections import OrderedDict
 from copy import deepcopy
 import logging
@@ -24,81 +24,136 @@ def parse_metadata(sample_list):
     return metadata
 
 
-def parse_sample_list(sample_sheet_file, run_data_directory_file_list):
+def verify_sample_sheet_file_names_in_file_list(sample_sheet_file, run_data_directory_file_list):
     """
-    Creates a list of all sample data in the sample_sheet_file
-    Verifies data is valid for uploading
+    Given a sample sheet, and a list of files in a directory,
+    verify that all the files on the sheet exist in the file list
+
+    If a file is missing, a SampleSheetError is raised
 
     :param sample_sheet_file:
-    :param run_data_directory_file_list: list of all files
-    :return: list of Sample objects
+    :param run_data_directory_file_list:
+    :return:
     """
     sample_list = _parse_samples(sample_sheet_file)
 
-    data_dir = path.dirname(sample_sheet_file)
-
-    data_dir_file_list_full_path = []
-    for file_name in run_data_directory_file_list:
-        data_dir_file_list_full_path.append(path.join(path.abspath(data_dir), file_name))
-
-    has_paired_end_read = False
-    has_single_end_read = False
-
-    logging.info("Verifying data parsed from sample sheet {}".format(sample_sheet_file))
-
     for sample in sample_list:
-
         sample_dict = sample.get_uploadable_dict()
-
         paired_end_read = len(sample_dict['File_Reverse']) > 0
-        # keep track if we have both paired and single end reads
-        if paired_end_read:
-            has_paired_end_read = True
-        else:
-            has_single_end_read = True
 
-        # Check if file names are in the files we found in the directory
-        if ((sample_dict['File_Forward'] not in run_data_directory_file_list) and (
-                sample_dict['File_Forward'] not in data_dir_file_list_full_path)):
+        # Check if file names are in the files we found in the directory file list
+        if sample_dict['File_Forward'] not in run_data_directory_file_list:
             raise exceptions.SampleSheetError(
-                ("Your sample sheet is malformed. {} Does not match any file in the directory {}"
-                 "".format(sample_dict['File_Forward'], data_dir)),
+                ("Your sample sheet is malformed. {} Does not match any file list in sample sheet file"
+                 "".format(sample_dict['File_Forward'])),
                 sample_sheet_file
             )
-        if ((paired_end_read and sample_dict['File_Reverse'] not in run_data_directory_file_list) and (
-                paired_end_read and sample_dict['File_Reverse'] not in data_dir_file_list_full_path)):
+        if paired_end_read and sample_dict['File_Reverse'] not in run_data_directory_file_list:
             raise exceptions.SampleSheetError(
-                ("Your sample sheet is malformed. {} Does not match any file in the directory {}"
-                 "".format(sample_dict['File_Reverse'], data_dir)),
+                ("Your sample sheet is malformed. {} Does not match any file list in sample sheet file"
+                 "".format(sample_dict['File_Reverse'])),
                 sample_sheet_file
             )
 
+
+def build_sample_list_from_sample_sheet_with_abs_path(sample_sheet_file):
+    """
+    Create a list of Sample objects, where each SequenceFile object has an absolute file path
+
+    :param sample_sheet_file:
+    :return:
+    """
+    sample_list = _parse_samples(sample_sheet_file)
+    # Data directory is used if file names on sample sheet are not absolute paths (in directory files)
+    data_dir = path.dirname(sample_sheet_file)
+    sample_sheet_dir_file_list = common.get_file_list(data_dir)
+
+    for sample in sample_list:
+        sample_dict = sample.get_uploadable_dict()
+        paired_end_read = len(sample_dict['File_Reverse']) > 0
+
         # create file list of full paths
         file_list = []
-        # Add the dir to each file to create the full path
-        if sample_dict['File_Forward'] not in data_dir_file_list_full_path:
-            sample_dict['File_Forward'] = path.join(data_dir, sample_dict['File_Forward'])
+        # If file is not an abspath already, make it an abspath from filename + data dir
+        if path.isabs(sample_dict['File_Forward']):
             file_list.append(sample_dict['File_Forward'])
-        if paired_end_read and sample_dict['File_Reverse'] not in data_dir_file_list_full_path:
-            sample_dict['File_Reverse'] = path.join(data_dir, sample_dict['File_Reverse'])
-            file_list.append(sample_dict['File_Reverse'])
+        elif sample_dict['File_Forward'] in sample_sheet_dir_file_list:
+            sample_dict['File_Forward'] = path.join(path.abspath(data_dir), sample_dict['File_Forward'])
+
+            file_list.append(sample_dict['File_Forward'])
+        else:
+            raise exceptions.SampleSheetError(
+                ("Your sample sheet is malformed. {} Does not match any file in the directory {}"
+                 "".format(sample_dict['File_Forward'], data_dir)),
+                sample_sheet_file)
+
+        # reverse file is same as for forward file
+        if paired_end_read:
+            if path.isabs(sample_dict['File_Reverse']):
+                file_list.append(sample_dict['File_Reverse'])
+            elif sample_dict['File_Reverse'] in sample_sheet_dir_file_list:
+                sample_dict['File_Reverse'] = path.join(path.abspath(data_dir), sample_dict['File_Reverse'])
+                file_list.append(sample_dict['File_Reverse'])
+            else:
+                raise exceptions.SampleSheetError(
+                    ("Your sample sheet is malformed. {} Does not match any file in the directory {}"
+                     "".format(sample_dict['File_Reverse'], data_dir)),
+                    sample_sheet_file)
 
         # Create sequence file object and attach to sample
         sq = model.SequenceFile(file_list=file_list)
         sample.sequence_file = deepcopy(sq)
 
-    # Verify we don't have both single end and paired end reads
-    if has_single_end_read and has_paired_end_read:
-        raise exceptions.SampleSheetError(
-            ("Your sample sheet is malformed. "
-             "SampleSheet cannot have both paired end and single end runs. "
-             "Make sure all samples are either paired or single."),
-            sample_sheet_file
-        )
+    return sample_list
+
+
+def build_sample_list_from_sample_sheet_no_verify(sample_sheet_file):
+    """
+    Create a list of Sample objects, file existence is not verified before SequenceFile is created
+    this is used when a pre-generated file list is used (e.g. cloud deployment)
+
+    :param sample_sheet_file:
+    :return:
+    """
+    sample_list = _parse_samples(sample_sheet_file)
+
+    for sample in sample_list:
+
+        sample_dict = sample.get_uploadable_dict()
+        # create file list
+        file_list = [sample_dict['File_Forward']]
+
+        # if paired end add file to file list
+        paired_end_read = len(sample_dict['File_Reverse']) > 0
+        if paired_end_read:
+            file_list.append(sample_dict['File_Reverse'])
+
+        # Create sequence file object and attach to sample
+        sq = model.SequenceFile(file_list=file_list)
+        sample.sequence_file = deepcopy(sq)
 
     return sample_list
 
 
+def only_single_or_paired_in_sample_list(sample_list):
+    """
+    Given a list of Sample objects, verifies there are only one type (single end or paired end)
+
+    :param sample_list:
+    :return: Boolean
+    """
+    has_single = False
+    has_paired = False
+
+    for sample in sample_list:
+        if sample.sequence_file.is_paired_end():
+            has_paired = True
+        else:
+            has_single = True
+
+    return not (has_single and has_paired)
+
+
 def _parse_samples(sample_sheet_file):
 
     """
@@ -176,8 +231,6 @@ def _parse_samples(sample_sheet_file):
 
             sample_dict[key] = value
 
-        sample_key_list = ['Sample_Name', 'Project_ID', 'File_Forward', 'File_Reverse']
-
         new_sample_dict = deepcopy(sample_dict)
         new_sample_name = new_sample_dict['Sample_Name']
         new_sample_project = new_sample_dict['Project_ID']
diff --git a/scripts/update_version.sh b/scripts/update_version.sh
old mode 100644
new mode 100755
diff --git a/setup.py b/setup.py
index 91be32c..2380857 100644
--- a/setup.py
+++ b/setup.py
@@ -6,7 +6,7 @@ with open("README.md", "r") as fh:
 
 setuptools.setup(
     name='iridauploader',
-    version='0.4.2',
+    version='0.4.3',
     description='IRIDA uploader: upload NGS data to IRIDA system',
     url='https://https://github.com/phac-nml/irida-uploader',
     author='Jeffrey Thiessen',
diff --git a/windows-gui-installer.cfg b/windows-gui-installer.cfg
index 24e7ab8..6f6d943 100644
--- a/windows-gui-installer.cfg
+++ b/windows-gui-installer.cfg
@@ -1,6 +1,6 @@
 [Application]
 name=IRIDA Sequence Uploader GUI
-version=0.4.2
+version=0.4.3
 entry_point=upload_gui:main
 icon=iridauploader/gui/images/icon.ico
 # Uncomment this to have a console show alongside the application
diff --git a/windows-installer.cfg b/windows-installer.cfg
index ec60e42..01dc2bb 100644
--- a/windows-installer.cfg
+++ b/windows-installer.cfg
@@ -1,6 +1,6 @@
 [Application]
 name=IRIDA Sequence Uploader
-version=0.4.2
+version=0.4.3
 entry_point=upload_run:main
 icon=iridauploader/gui/images/icon.ico
 # We need to set this to get a console:


Test patch:
diff --git a/iridauploader/tests/parsers/directory/test_sample_parser.py b/iridauploader/tests/parsers/directory/test_sample_parser.py
index d338af4..ca63a0e 100644
--- a/iridauploader/tests/parsers/directory/test_sample_parser.py
+++ b/iridauploader/tests/parsers/directory/test_sample_parser.py
@@ -1,9 +1,7 @@
 import unittest
-from collections import OrderedDict
 from os import path
 from unittest.mock import patch
 
-import iridauploader.parsers as parsers
 import iridauploader.parsers.directory.sample_parser as sample_parser
 from iridauploader.parsers.exceptions import SampleSheetError
 import iridauploader.model as model
@@ -13,7 +11,42 @@ if len(path_to_module) == 0:
     path_to_module = '.'
 
 
-class TestParseSampleList(unittest.TestCase):
+class TestVerifySampleSheetFileNamesInFileList(unittest.TestCase):
+    """
+    test file existence verification function
+    """
+
+    def setUp(self):
+        print("\nStarting " + self.__module__ + ": " + self._testMethodName)
+
+    def test_valid(self):
+        sheet_file = path.join(path_to_module, "fake_dir_data",
+                               "SampleList_simple.csv")
+
+        run_data_directory_file_list = ["file_1.fastq.gz", "file_2.fastq.gz"]
+
+        sample_parser.verify_sample_sheet_file_names_in_file_list(sheet_file, run_data_directory_file_list)
+
+    def test_file_names_do_not_match_paired_end(self):
+        sheet_file = path.join(path_to_module, "fake_dir_data",
+                               "SampleList_simple.csv")
+
+        run_data_directory_file_list = ["file_1.fastq.gz", "file_a.fastq.gz"]
+
+        with self.assertRaises(SampleSheetError):
+            sample_parser.verify_sample_sheet_file_names_in_file_list(sheet_file, run_data_directory_file_list)
+
+    def test_file_names_do_not_match_single_end(self):
+        sheet_file = path.join(path_to_module, "fake_dir_data",
+                               "no_reverse.csv")
+
+        run_data_directory_file_list = ["file_a.fastq.gz"]
+
+        with self.assertRaises(SampleSheetError):
+            sample_parser.verify_sample_sheet_file_names_in_file_list(sheet_file, run_data_directory_file_list)
+
+
+class TestBuildSampleListFromSampleSheetWithAbsPath(unittest.TestCase):
     """
     test parsing the list of samples from a sample sheet
     """
@@ -34,8 +67,7 @@ class TestParseSampleList(unittest.TestCase):
         file_path_2 = path.join(path_to_module,
                                 "fake_dir_data", "file_2.fastq.gz")
 
-        run_data_directory_file_list = ["file_1.fastq.gz", "file_2.fastq.gz"]
-        res = sample_parser.parse_sample_list(sheet_file, run_data_directory_file_list)
+        res = sample_parser.build_sample_list_from_sample_sheet_with_abs_path(sheet_file)
 
         # Check we have 1 sample
         self.assertEqual(len(res), 1)
@@ -44,6 +76,8 @@ class TestParseSampleList(unittest.TestCase):
         self.assertEqual(res[0].get_uploadable_dict()["sample_project"], "75")
         self.assertEqual(res[0].get_uploadable_dict()["File_Forward"], "file_1.fastq.gz")
         self.assertEqual(res[0].get_uploadable_dict()["File_Reverse"], "file_2.fastq.gz")
+        self.assertEqual(res[0].sequence_file.file_list[0], file_path_1)
+        self.assertEqual(res[0].sequence_file.file_list[1], file_path_2)
 
     @patch("iridauploader.parsers.directory.sample_parser._parse_samples")
     def test_valid_full_file_path(self, mock_parse_samples):
@@ -74,8 +108,7 @@ class TestParseSampleList(unittest.TestCase):
 
         mock_parse_samples.return_value = sample_list
 
-        run_data_directory_file_list = ["file_1.fastq.gz", "file_2.fastq.gz"]
-        res = sample_parser.parse_sample_list(sheet_file, run_data_directory_file_list)
+        res = sample_parser.build_sample_list_from_sample_sheet_with_abs_path(sheet_file)
 
         mock_parse_samples.assert_called_with(sheet_file)
         # Check we have 1 sample
@@ -85,6 +118,8 @@ class TestParseSampleList(unittest.TestCase):
         self.assertEqual(res[0].get_uploadable_dict()["sample_project"], "75")
         self.assertEqual(res[0].get_uploadable_dict()["File_Forward"], path.abspath(file_path_1))
         self.assertEqual(res[0].get_uploadable_dict()["File_Reverse"], path.abspath(file_path_2))
+        self.assertEqual(res[0].sequence_file.file_list[0], file_path_1)
+        self.assertEqual(res[0].sequence_file.file_list[1], file_path_2)
 
     def test_no_forward_read(self):
         """
@@ -95,7 +130,7 @@ class TestParseSampleList(unittest.TestCase):
         file_path = path.join(directory, "list_no_forward.csv")
 
         with self.assertRaises(SampleSheetError):
-            res = sample_parser.parse_sample_list(file_path, directory)
+            res = sample_parser.build_sample_list_from_sample_sheet_with_abs_path(file_path)
 
     def test_no_reverse_read(self):
         """
@@ -106,14 +141,77 @@ class TestParseSampleList(unittest.TestCase):
         file_path = path.join(directory, "list_no_reverse.csv")
 
         with self.assertRaises(SampleSheetError):
-            res = sample_parser.parse_sample_list(file_path, directory)
+            res = sample_parser.build_sample_list_from_sample_sheet_with_abs_path(file_path)
+
+
+class TestBuildSampleListFromSampleSheetNoVerify(unittest.TestCase):
+    """
+    test parsing the list of samples from a sample sheet
+    These samplelists are built without verification that the files exist
+    But we still check the positive test cases
+    """
+
+    def setUp(self):
+        print("\nStarting " + self.__module__ + ": " + self._testMethodName)
+
+    def test_valid(self):
+        """
+        Given a valid sample sheet, parse correctly
+        :return:
+        """
+        sheet_file = path.join(path_to_module, "fake_dir_data",
+                               "SampleList_simple.csv")
+
+        file_name_1 = "file_1.fastq.gz"
+        file_name_2 = "file_2.fastq.gz"
+
+        res = sample_parser.build_sample_list_from_sample_sheet_no_verify(sheet_file)
+
+        # Check we have 1 sample
+        self.assertEqual(len(res), 1)
+        # Check if data is correct
+        self.assertEqual(res[0].sample_name, "my-sample-1")
+        self.assertEqual(res[0].get_uploadable_dict()["sample_project"], "75")
+        self.assertEqual(res[0].get_uploadable_dict()["File_Forward"], file_name_1)
+        self.assertEqual(res[0].get_uploadable_dict()["File_Reverse"], file_name_2)
+        self.assertEqual(res[0].sequence_file.file_list[0], file_name_1)
+        self.assertEqual(res[0].sequence_file.file_list[1], file_name_2)
+
+
+class TestOnlySingleOrPairedInSampleList(unittest.TestCase):
+    """
+    test boolean of if there is only one of paired or single end files in a list of samples
+    """
+
+    def setUp(self):
+        print("\nStarting " + self.__module__ + ": " + self._testMethodName)
 
     def test_mixed_paired_and_single_reads(self):
         directory = path.join(path_to_module, "fake_dir_data")
         file_path = path.join(directory, "list_mixed.csv")
 
-        with self.assertRaises(SampleSheetError):
-            res = sample_parser.parse_sample_list(file_path, directory)
+        sample_list = sample_parser.build_sample_list_from_sample_sheet_with_abs_path(file_path)
+        res = sample_parser.only_single_or_paired_in_sample_list(sample_list)
+
+        self.assertFalse(res)
+
+    def test_only_single_end_reads(self):
+        directory = path.join(path_to_module, "fake_dir_data")
+        file_path = path.join(directory, "no_reverse.csv")
+
+        sample_list = sample_parser.build_sample_list_from_sample_sheet_with_abs_path(file_path)
+        res = sample_parser.only_single_or_paired_in_sample_list(sample_list)
+
+        self.assertTrue(res)
+
+    def test_only_paired_end_reads(self):
+        directory = path.join(path_to_module, "fake_dir_data")
+        file_path = path.join(directory, "SampleList.csv")
+
+        sample_list = sample_parser.build_sample_list_from_sample_sheet_with_abs_path(file_path)
+        res = sample_parser.only_single_or_paired_in_sample_list(sample_list)
+
+        self.assertTrue(res)
 
 
 class TestParseSamples(unittest.TestCase):