From 19d1d928f063fa268b86798e2627fcf84954601d Mon Sep 17 00:00:00 2001
From: Nicolai-vKuegelgen <nicolai.von-kuegelgen@bih-charite.de>
Date: Mon, 19 Aug 2024 13:06:05 +0200
Subject: [PATCH 1/8] feat: presets for cubi-tk sodar ingest-fastq (#232 )

---
 cubi_tk/sodar/ingest_fastq.py    | 104 +++++++++++++-----
 tests/test_sodar_ingest_fastq.py | 180 +++++++++++++++++++++++++++++--
 2 files changed, 249 insertions(+), 35 deletions(-)

diff --git a/cubi_tk/sodar/ingest_fastq.py b/cubi_tk/sodar/ingest_fastq.py
index 6f213f2a..a7081d64 100644
--- a/cubi_tk/sodar/ingest_fastq.py
+++ b/cubi_tk/sodar/ingest_fastq.py
@@ -30,17 +30,40 @@
 formatter = logzero.LogFormatter(fmt="%(message)s")
 output_logger = logzero.setup_logger(formatter=formatter)
 
-DEFAULT_SRC_REGEX = (
-    r"(.*/)?(?P<sample>.+?)"
-    r"(?:_S[0-9]+)?"
-    r"(?:_(?P<lane>L[0-9]+?))?"
-    r"(?:_(?P<mate>R[0-9]+?))?"
-    r"(?:_(?P<batch>[0-9]+?))?"
-    r"\.f(?:ast)?q\.gz"
-)
-
-#: Default value for --dest-pattern
-DEFAULT_DEST_PATTERN = r"{collection_name}/raw_data/{date}/{filename}"
+SRC_REGEX_PRESETS = {
+    "default": (
+        r"(.*/)?(?P<sample>.+?)"
+        r"(?:_S[0-9]+)?"
+        r"(?:_(?P<lane>L[0-9]+?))?"
+        r"(?:_(?P<mate>R[0-9]+?))?"
+        r"(?:_(?P<batch>[0-9]+?))?"
+        r"\.f(?:ast)?q\.gz"
+    ),
+    "digestiflow": (
+        r"(.*/)?(?P<flowcell>[A-Z0-9]{9,10}?)/"
+        r"(?P<lane>L[0-9]{3}?)/"
+        r"(?P<sample>.+?)_"
+        r"S[0-9]+_L[0-9]{3}_R[0-9]_[0-9]{3}"
+        r"\.fastq\.gz"
+    ),
+    "ONT": (
+        r"(.*/)?"
+        r"[0-9]{8}_"  # Date
+        # Sample could be <ProjectID>_<LibID>_<SampleID>, but this is not given and may change between projects
+        r"(?P<sample>[a-zA-Z0-9_-]+?)/"
+        # RunID is <date>_<time>_<position>_<flowcellID>_<hash>
+        # Flowcells can be re-used, so taking the whole thing for uniqueness is best
+        r"(?P<RunID>[0-9]{8}_[0-9]+_[A-Z0-9]+_[A-Z0-9]+_[0-9a-z]+?)/"
+        r"(?:(?P<subfolder>[a-z0-9_]+/))?"
+        r".+\.(bam|pod5|txt|json)"
+    ),
+}
+
+DEST_PATTERN_PRESETS = {
+    "default": r"{collection_name}/raw_data/{date}/{filename}",
+    "digestiflow": r"{collection_name}/raw_data/{flowcell}/{filename}",
+    "ONT": r"{collection_name}/raw_data/{RunID}/{subfolder}{filename}",
+}
 
 #: Default number of parallel transfers.
 DEFAULT_NUM_TRANSFERS = 8
@@ -82,7 +105,11 @@ class SodarIngestFastq(SnappyItransferCommandBase):
 
     def __init__(self, args):
         super().__init__(args)
-        self.dest_pattern_fields = set(re.findall(r"(?<={).+?(?=})", self.args.remote_dir_pattern))
+        if self.args.remote_dir_pattern:
+            self.remote_dir_pattern = self.args.remote_dir_pattern
+        else:
+            self.remote_dir_pattern = DEST_PATTERN_PRESETS[self.args.preset]
+        self.dest_pattern_fields = set(re.findall(r"(?<={).+?(?=})", self.remote_dir_pattern))
 
     @classmethod
     def setup_argparse(cls, parser: argparse.ArgumentParser) -> None:
@@ -128,20 +155,29 @@ def setup_argparse(cls, parser: argparse.ArgumentParser) -> None:
             action="store_true",
             help="After files are transferred to SODAR, it will proceed with validation and move.",
         )
+        parser.add_argument(
+            "--preset",
+            default="default",
+            choices=DEST_PATTERN_PRESETS.keys(),
+            help=f"Use predefined values for regular expression to find local files (--src-regex) and pattern to for "
+            f"constructing remote file paths.\nDefault src-regex: {SRC_REGEX_PRESETS['default']}.\n"
+            f"Default --remote-dir-pattern: {DEST_PATTERN_PRESETS['default']}.",
+        )
         parser.add_argument(
             "--src-regex",
-            default=DEFAULT_SRC_REGEX,
-            help=f"Regular expression to use for matching input fastq files, default: {DEFAULT_SRC_REGEX}. "
-            "All capture groups can be used for --remote-dir-pattern, but only 'sample' is used by default. "
-            "Only this regex controls which files are ingested, so other files than fastq.gz can be used too.",
+            default=None,
+            help="Manually defined regular expression to use for matching input fastq files. Takes precedence over "
+            "--preset.  This regex controls which files are ingested, so it can be used for any file type. "
+            "Any named capture group in the regex can be used with --remote-dir-pattern. The 'sample' group is "
+            "used to set irods collection names (as-is or via --match-column).",
         )
         parser.add_argument(
             "--remote-dir-pattern",
-            default=DEFAULT_DEST_PATTERN,
-            help=f"Pattern to use for constructing remote pattern, default: {DEFAULT_DEST_PATTERN}. "
-            "'collection_name' is the target iRODS collection and will be filled with the (-m regex modified) "
-            "'sample' unless --match-column is not used to fill it from the assay table. Any capture group of the "
-            "src-regex ('sample', 'lane', ...) can be used along with 'date' and 'filename'.",
+            default=None,
+            help="Manually defined pattern to use for constructing remote file paths. Takes precedence over "
+            "--preset. 'collection_name' is the target iRODS collection and will be filled with the (-m regex "
+            "modified) 'sample', or if --match-column is used with teh corresponding value from the  assay table. "
+            "Any capture group of the src-regex ('sample', 'lane', ...) can be used along with 'date' and 'filename'.",
         )
         parser.add_argument(
             "--match-column",
@@ -417,6 +453,12 @@ def build_jobs(self, library_names=None):
         folders = self.download_webdav(self.args.sources)
         transfer_jobs = []
 
+        if self.args.src_regex:
+            use_regex = self.args.src_regex
+        else:
+            use_regex = SRC_REGEX_PRESETS[self.args.preset]
+        # logger.debug(f"Using regex: {use_regex}")
+
         for folder in folders:
             for path in glob.iglob(f"{folder}/**/*", recursive=True):
                 real_path = os.path.realpath(path)
@@ -432,11 +474,10 @@ def build_jobs(self, library_names=None):
                 ):  # pragma: nocover
                     raise MissingFileException("Missing file %s" % (real_path + ".md5"))
 
-                m = re.match(self.args.src_regex, path)
+                # logger.debug(f"Checking file: {path}")
+                m = re.match(use_regex, path)
                 if m:
-                    logger.debug(
-                        "Matched %s with regex %s: %s", path, self.args.src_regex, m.groupdict()
-                    )
+                    logger.debug("Matched %s with regex %s: %s", path, use_regex, m.groupdict())
                     match_wildcards = dict(
                         item
                         for item in m.groupdict(default="").items()
@@ -449,9 +490,7 @@ def build_jobs(self, library_names=None):
                         sample_name = re.sub(m_pat, r_pat, sample_name)
 
                     try:
-                        remote_file = pathlib.Path(
-                            lz_irods_path
-                        ) / self.args.remote_dir_pattern.format(
+                        remote_file = pathlib.Path(lz_irods_path) / self.remote_dir_pattern.format(
                             # Removed the `+ self.args.add_suffix` here, since adding anything after the file extension is a bad idea
                             filename=pathlib.Path(path).name,
                             date=self.args.remote_dir_date,
@@ -495,6 +534,15 @@ def execute(self) -> typing.Optional[int]:
 
         lz_uuid, transfer_jobs = self.build_jobs()
         transfer_jobs = sorted(transfer_jobs, key=lambda x: x.path_local)
+        # Exit early if no files were found/matched
+        if not transfer_jobs:
+            if self.args.src_regex:
+                used_regex = self.args.src_regex
+            else:
+                used_regex = SRC_REGEX_PRESETS[self.args.preset]
+
+            logger.warning("No matching files were found!\nUsed regex: %s", used_regex)
+            return None
 
         if self.fix_md5_files:
             transfer_jobs = self._execute_md5_files_fix(transfer_jobs)
diff --git a/tests/test_sodar_ingest_fastq.py b/tests/test_sodar_ingest_fastq.py
index 9ba0c8bd..082b6d9a 100644
--- a/tests/test_sodar_ingest_fastq.py
+++ b/tests/test_sodar_ingest_fastq.py
@@ -46,22 +46,66 @@ def test_run_sodar_ingest_fastq_nothing(capsys):
     assert res.err
 
 
-def test_run_sodar_ingest_fastq_src_regex():
-    from cubi_tk.sodar.ingest_fastq import DEFAULT_SRC_REGEX
+def test_run_sodar_ingest_fastq_preset_definitions():
+    from cubi_tk.sodar.ingest_fastq import DEST_PATTERN_PRESETS, SRC_REGEX_PRESETS
 
+    regexes = SRC_REGEX_PRESETS.keys()
+    patterns = DEST_PATTERN_PRESETS.keys()
+
+    # Check that all presets exits for both regex & pattern
+    assert sorted(regexes) == sorted(patterns)
+
+    # Check that all presets are not empty
+    for preset in regexes:
+        assert SRC_REGEX_PRESETS[preset]
+        assert DEST_PATTERN_PRESETS[preset]
+
+
+def test_run_sodar_ingest_fastq_default_preset_regex():
+    from cubi_tk.sodar.ingest_fastq import SRC_REGEX_PRESETS
+
+    ## Test default regex
     # Collection of example filenames and the expected {sample} value the regex should capture
     test_filenames = {
         "Sample1-N1-RNA1-RNA_seq1.fastq.gz": "Sample1-N1-RNA1-RNA_seq1",
         "P1234_Samplename_S14_L006_R2_001.fastq.gz": "P1234_Samplename",
         "P1234_Samplename2_R1.fastq.gz": "P1234_Samplename2",
     }
-
     for test_filename, expected_sample in test_filenames.items():
-        res = re.match(DEFAULT_SRC_REGEX, test_filename)
+        res = re.match(SRC_REGEX_PRESETS["default"], test_filename)
         assert res is not None
         assert res.groupdict()["sample"] == expected_sample
 
 
+def test_run_sodar_ingest_fastq_ont_preset_regex():
+    from cubi_tk.sodar.ingest_fastq import SRC_REGEX_PRESETS
+
+    test_filenames = {
+        "fake_base_path/20240101_A0000_sample1/20240101_0000_A1_AB12345_000xyz/bam_fail/"
+        "AB12345_000xyz_pass_1c1234_0.bam": (
+            "A0000_sample1",
+            "20240101_0000_A1_AB12345_000xyz",
+            "bam_fail/",
+        ),
+        "fake_base_path/20240101_A0000_sample1/20240101_0000_A1_AB12345_000xyz/"
+        "final_summary_AB12345_000xyz_1c1234_0.txt": (
+            "A0000_sample1",
+            "20240101_0000_A1_AB12345_000xyz",
+            None,
+        ),
+    }
+    for test_filename, expected_res in test_filenames.items():
+        res = re.match(SRC_REGEX_PRESETS["ONT"], test_filename)
+        assert res is not None
+        groups = res.groupdict()
+        assert groups["sample"] == expected_res[0]
+        assert groups["RunID"] == expected_res[1]
+        if expected_res[2]:
+            assert groups["subfolder"] == expected_res[2]
+        else:
+            assert groups["subfolder"] is None
+
+
 @patch("cubi_tk.sodar.ingest_fastq.api.samplesheet.retrieve")
 @patch("cubi_tk.sodar.ingest_fastq.api.samplesheet.export")
 def test_run_sodar_ingest_fastq_get_match_to_collection_mapping(mock_api_export, mock_api_retrieve):
@@ -128,7 +172,7 @@ def test_run_sodar_ingest_fastq_smoke_test(mocker, requests_mock):
     # --- setup arguments
     irods_path = "/irods/dest"
     landing_zone_uuid = "466ab946-ce6a-4c78-9981-19b79e7bbe86"
-    dest_path = "target/folder/"
+    # dest_path = "target/folder/"
     fake_base_path = "/base/path"
     argv = [
         "--verbose",
@@ -139,8 +183,8 @@ def test_run_sodar_ingest_fastq_smoke_test(mocker, requests_mock):
         "--sodar-api-token",
         "XXXX",
         "--yes",
-        "--remote-dir-pattern",
-        dest_path,
+        # "--remote-dir-pattern",
+        # dest_path,
         fake_base_path,
         landing_zone_uuid,
     ]
@@ -233,3 +277,125 @@ def test_run_sodar_ingest_fastq_smoke_test(mocker, requests_mock):
     ingestfastq = SodarIngestFastq(args)
     lz, actual = ingestfastq.build_jobs()
     assert len(actual) == len(fake_file_paths)
+
+
+def test_run_sodar_ingest_fastq_smoke_test_ont_preset(mocker, requests_mock):
+    # --- setup arguments
+    irods_path = "/irods/dest"
+    landing_zone_uuid = "466ab946-ce6a-4c78-9981-19b79e7bbe86"
+    # dest_path = "target/folder/"
+    fake_base_path = "/base/path"
+    argv = [
+        "--verbose",
+        "sodar",
+        "ingest-fastq",
+        "--num-parallel-transfers",
+        "0",
+        "--sodar-api-token",
+        "XXXX",
+        "--yes",
+        "--preset",
+        "ONT",
+        # "--remote-dir-pattern",
+        # dest_path,
+        fake_base_path,
+        landing_zone_uuid,
+    ]
+
+    parser, _subparsers = setup_argparse()
+    args = parser.parse_args(argv)
+
+    # Setup fake file system but only patch selected modules.  We cannot use the Patcher approach here as this would
+    # break biomedsheets.
+    fs = fake_filesystem.FakeFilesystem()
+    fake_os = fake_filesystem.FakeOsModule(fs)
+    fake_pl = fake_pathlib.FakePathlibModule(fs)
+
+    # --- add test files
+    fake_file_paths = []
+    date = "20240101"
+    project_sample_id = "A0000_sample{n}"
+    for sample_n in (1, 2, 3):
+        sample_path = project_sample_id.format(n=sample_n)
+        # date _ time _ positions _ ID _ hash
+        flowcell_id_hash = "AB12345_000xyz"
+        flowcellrun = f"{date}_0000_1A_{flowcell_id_hash}"
+        for file_pattern in (
+            f"{fake_base_path}/{date}_{sample_path}/{flowcellrun}/bam_fail/{flowcell_id_hash}_pass_{sample_n}c1234_0.bam",
+            f"{fake_base_path}/{date}_{sample_path}/{flowcellrun}/bam_pass/{flowcell_id_hash}_fail_{sample_n}c1234_0.bam",
+            f"{fake_base_path}/{date}_{sample_path}/{flowcellrun}/final_summary_{flowcell_id_hash}_{sample_n}c1234_0.txt",
+            f"{fake_base_path}/{date}_{sample_path}/{flowcellrun}/pod5/{flowcell_id_hash}_{sample_n}c1234_0.pod5",
+            f"{fake_base_path}/{date}_{sample_path}/{flowcellrun}/report_{flowcell_id_hash}.html",
+            f"{fake_base_path}/{date}_{sample_path}/{flowcellrun}/report_{flowcell_id_hash}.json",
+            f"{fake_base_path}/{date}_{sample_path}/{flowcellrun}/sequencing_summary_{flowcell_id_hash}_{sample_n}c1234_0.txt",
+        ):
+            for ext in ("", ".md5"):
+                fake_file_paths.append(file_pattern + ext)
+                fs.create_file(fake_file_paths[-1])
+
+    # Remove MD5 file for sample 1 fail bam, so it is recreated.
+    fs.remove(fake_file_paths[3])
+
+    # --- mock modules
+    mocker.patch("glob.os", fake_os)
+    mocker.patch("cubi_tk.snappy.itransfer_common.os", fake_os)
+    mocker.patch(
+        "cubi_tk.snappy.itransfer_common.SnappyItransferCommandBase.get_sodar_info",
+        my_get_sodar_info,
+    )
+
+    mock_check_output = mock.MagicMock(return_value=0)
+    mocker.patch("cubi_tk.irods_common.iRODSTransfer.put", mock_check_output)
+
+    mock_check_call = mock.MagicMock(return_value=0)
+    mocker.patch("cubi_tk.snappy.itransfer_common.check_call", mock_check_call)
+    mocker.patch("cubi_tk.sodar.ingest_fastq.check_call", mock_check_call)
+
+    mocker.patch("cubi_tk.sodar.ingest_fastq.pathlib", fake_pl)
+    mocker.patch("cubi_tk.sodar.ingest_fastq.os", fake_os)
+
+    fake_open = fake_filesystem.FakeFileOpen(fs)
+    mocker.patch("cubi_tk.snappy.itransfer_common.open", fake_open)
+    mocker.patch("cubi_tk.sodar.ingest_fastq.open", fake_open)
+
+    # necessary because independent test fail
+    mock_value = mock.MagicMock()
+    mocker.patch("cubi_tk.sodar.ingest_fastq.Value", mock_value)
+    mocker.patch("cubi_tk.snappy.itransfer_common.Value", mock_value)
+
+    # requests mock
+    return_value = dict(
+        assay="",
+        config_data="",
+        configuration="",
+        date_modified="",
+        description="",
+        irods_path=irods_path,
+        project="",
+        sodar_uuid="",
+        status="",
+        status_info="",
+        title="",
+        user=dict(sodar_uuid="", username="", name="", email=""),
+    )
+    url = os.path.join(args.sodar_url, "landingzones", "api", "retrieve", args.destination)
+    requests_mock.register_uri("GET", url, text=json.dumps(return_value))
+
+    # --- run tests
+    res = main(argv)
+
+    assert not res
+
+    assert mock_check_call.call_count == 1
+    assert mock_check_call.call_args[0] == (["md5sum", "AB12345_000xyz_fail_1c1234_0.bam"],)
+
+    # The upload logic for multiple transfers/files has been moved into the iRODScommon classes
+    # We just need one call to that here
+    assert mock_check_output.call_count == 1
+
+    # Test that the TransferJob contain all files, except html (3x2 for md5s)
+    parser, _subparsers = setup_argparse()
+    args = parser.parse_args(argv)
+    ingestfastq = SodarIngestFastq(args)
+    lz, actual = ingestfastq.build_jobs()
+    assert len(actual) == len(fake_file_paths) - 6

From ea34f7eab17be5ad47c8c03f1daefb435f54dab4 Mon Sep 17 00:00:00 2001
From: Till Hartmann <till.hartmann@tu-dortmund.de>
Date: Mon, 19 Aug 2024 14:18:59 +0200
Subject: [PATCH 2/8] chore: pin snappy version to v0.1.1 in requirements.txt
 (#237)

(cherry picked from commit 1e3d38c877eabf39b11cb14b4e9edb79d8a72bbe)
---
 requirements/test.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/requirements/test.txt b/requirements/test.txt
index 5bdd1d32..3338a96e 100644
--- a/requirements/test.txt
+++ b/requirements/test.txt
@@ -44,4 +44,4 @@ pyflakes
 setuptools ==65.6.3
 
 # needed for testing snappy workflow methods
-snappy-pipeline @ git+https://github.com/bihealth/snappy-pipeline
+snappy-pipeline @ git+https://github.com/bihealth/snappy-pipeline@v0.1.1

From ac66287de15362a4b708ec44e9a854ee9c189725 Mon Sep 17 00:00:00 2001
From: Nicolai-vKuegelgen <nicolai.von-kuegelgen@bih-charite.de>
Date: Thu, 22 Aug 2024 13:23:14 +0200
Subject: [PATCH 3/8] Code-rebview & test for digestiflow regex

---
 cubi_tk/sodar/ingest_fastq.py    | 14 +++++++++++---
 tests/test_sodar_ingest_fastq.py | 22 ++++++++++++++++++++++
 2 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/cubi_tk/sodar/ingest_fastq.py b/cubi_tk/sodar/ingest_fastq.py
index a7081d64..24728150 100644
--- a/cubi_tk/sodar/ingest_fastq.py
+++ b/cubi_tk/sodar/ingest_fastq.py
@@ -42,6 +42,7 @@
     "digestiflow": (
         r"(.*/)?(?P<flowcell>[A-Z0-9]{9,10}?)/"
         r"(?P<lane>L[0-9]{3}?)/"
+        r"(?:(?P<project>[A-Z][0-9]+_?))?"
         r"(?P<sample>.+?)_"
         r"S[0-9]+_L[0-9]{3}_R[0-9]_[0-9]{3}"
         r"\.fastq\.gz"
@@ -176,7 +177,7 @@ def setup_argparse(cls, parser: argparse.ArgumentParser) -> None:
             default=None,
             help="Manually defined pattern to use for constructing remote file paths. Takes precedence over "
             "--preset. 'collection_name' is the target iRODS collection and will be filled with the (-m regex "
-            "modified) 'sample', or if --match-column is used with teh corresponding value from the  assay table. "
+            "modified) 'sample', or if --match-column is used with the corresponding value from the  assay table. "
             "Any capture group of the src-regex ('sample', 'lane', ...) can be used along with 'date' and 'filename'.",
         )
         parser.add_argument(
@@ -259,6 +260,13 @@ def check_args(self, args):
                 )
                 res = 1
 
+        if args.src_regex and args.remote_dir_pattern and args.preset != "default":
+            logger.error(
+                "Using both --src-regex and --remote-dir-pattern at the same time overwrites all values defined "
+                "by --preset. Please drop the use of --preset or at least one of the other manual definitions."
+            )
+            res = 1
+
         return res
 
     def get_project_uuid(self, lz_uuid: str):
@@ -454,9 +462,9 @@ def build_jobs(self, library_names=None):
         transfer_jobs = []
 
         if self.args.src_regex:
-            use_regex = self.args.src_regex
+            use_regex = re.compile(self.args.src_regex)
         else:
-            use_regex = SRC_REGEX_PRESETS[self.args.preset]
+            use_regex = re.compile(SRC_REGEX_PRESETS[self.args.preset])
         # logger.debug(f"Using regex: {use_regex}")
 
         for folder in folders:
diff --git a/tests/test_sodar_ingest_fastq.py b/tests/test_sodar_ingest_fastq.py
index 082b6d9a..3cafa46b 100644
--- a/tests/test_sodar_ingest_fastq.py
+++ b/tests/test_sodar_ingest_fastq.py
@@ -76,6 +76,28 @@ def test_run_sodar_ingest_fastq_default_preset_regex():
         assert res is not None
         assert res.groupdict()["sample"] == expected_sample
 
+def test_run_sodar_ingest_fastq_digestiflow_preset_regex():
+    from cubi_tk.sodar.ingest_fastq import SRC_REGEX_PRESETS
+
+    ## Test default regex
+    # Collection of example filenames and the expected {sample} value the regex should capture
+    pattern = "240101_XY01234_0000_B{flowcell}/A0000_{sample}/{flowcell}/{lane}/A0000_{sample}_S1_{lane}_R1_001.fastq.gz"
+    samples = ("sample1", "sample2")
+    flowcells = ("AB123XY456", "CD678LT000")
+    lanes = ("L001", "L002")
+    test_filenames = dict()
+    for flowcell in flowcells:
+        for sample in samples:
+            for lane in lanes:
+                test_filename = pattern.format(flowcell=flowcell, sample=sample, lane=lane)
+                test_filenames[test_filename] = (sample, flowcell)
+
+    for test_filename, expected in test_filenames.items():
+        res = re.match(SRC_REGEX_PRESETS["digestiflow"], test_filename)
+        assert res is not None
+        assert res.groupdict()["sample"] == expected[0]
+        assert res.groupdict()["flowcell"] == expected[1]
+
 
 def test_run_sodar_ingest_fastq_ont_preset_regex():
     from cubi_tk.sodar.ingest_fastq import SRC_REGEX_PRESETS

From 5aacc358ec7fb96aa1ad1accff7eeced2bba89b4 Mon Sep 17 00:00:00 2001
From: Nicolai-vKuegelgen <nicolai.von-kuegelgen@bih-charite.de>
Date: Thu, 22 Aug 2024 13:23:14 +0200
Subject: [PATCH 4/8] Code-review & test for digestiflow regex

---
 tests/test_sodar_ingest_fastq.py | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/tests/test_sodar_ingest_fastq.py b/tests/test_sodar_ingest_fastq.py
index 3cafa46b..8068ed0e 100644
--- a/tests/test_sodar_ingest_fastq.py
+++ b/tests/test_sodar_ingest_fastq.py
@@ -99,6 +99,29 @@ def test_run_sodar_ingest_fastq_digestiflow_preset_regex():
         assert res.groupdict()["flowcell"] == expected[1]
 
 
+def test_run_sodar_ingest_fastq_digestiflow_preset_regex():
+    from cubi_tk.sodar.ingest_fastq import SRC_REGEX_PRESETS
+
+    ## Test default regex
+    # Collection of example filenames and the expected {sample} value the regex should capture
+    pattern = "240101_XY01234_0000_B{flowcell}/A0000_{sample}/{flowcell}/{lane}/A0000_{sample}_S1_{lane}_R1_001.fastq.gz"
+    samples = ("sample1", "sample2")
+    flowcells = ("AB123XY456", "CD678LT000")
+    lanes = ("L001", "L002")
+    test_filenames = dict()
+    for flowcell in flowcells:
+        for sample in samples:
+            for lane in lanes:
+                test_filename = pattern.format(flowcell=flowcell, sample=sample, lane=lane)
+                test_filenames[test_filename] = (sample, flowcell)
+
+    for test_filename, expected in test_filenames.items():
+        res = re.match(SRC_REGEX_PRESETS["digestiflow"], test_filename)
+        assert res is not None
+        assert res.groupdict()["sample"] == expected[0]
+        assert res.groupdict()["flowcell"] == expected[1]
+
+
 def test_run_sodar_ingest_fastq_ont_preset_regex():
     from cubi_tk.sodar.ingest_fastq import SRC_REGEX_PRESETS
 

From 10dc4eb619cbec201fdedc4a7e481e69fac84cb6 Mon Sep 17 00:00:00 2001
From: Nicolai-vKuegelgen <nicolai.von-kuegelgen@bih-charite.de>
Date: Thu, 22 Aug 2024 13:32:40 +0200
Subject: [PATCH 5/8] lint no 2

---
 tests/test_sodar_ingest_fastq.py | 22 ----------------------
 1 file changed, 22 deletions(-)

diff --git a/tests/test_sodar_ingest_fastq.py b/tests/test_sodar_ingest_fastq.py
index 8068ed0e..4fa1236d 100644
--- a/tests/test_sodar_ingest_fastq.py
+++ b/tests/test_sodar_ingest_fastq.py
@@ -76,28 +76,6 @@ def test_run_sodar_ingest_fastq_default_preset_regex():
         assert res is not None
         assert res.groupdict()["sample"] == expected_sample
 
-def test_run_sodar_ingest_fastq_digestiflow_preset_regex():
-    from cubi_tk.sodar.ingest_fastq import SRC_REGEX_PRESETS
-
-    ## Test default regex
-    # Collection of example filenames and the expected {sample} value the regex should capture
-    pattern = "240101_XY01234_0000_B{flowcell}/A0000_{sample}/{flowcell}/{lane}/A0000_{sample}_S1_{lane}_R1_001.fastq.gz"
-    samples = ("sample1", "sample2")
-    flowcells = ("AB123XY456", "CD678LT000")
-    lanes = ("L001", "L002")
-    test_filenames = dict()
-    for flowcell in flowcells:
-        for sample in samples:
-            for lane in lanes:
-                test_filename = pattern.format(flowcell=flowcell, sample=sample, lane=lane)
-                test_filenames[test_filename] = (sample, flowcell)
-
-    for test_filename, expected in test_filenames.items():
-        res = re.match(SRC_REGEX_PRESETS["digestiflow"], test_filename)
-        assert res is not None
-        assert res.groupdict()["sample"] == expected[0]
-        assert res.groupdict()["flowcell"] == expected[1]
-
 
 def test_run_sodar_ingest_fastq_digestiflow_preset_regex():
     from cubi_tk.sodar.ingest_fastq import SRC_REGEX_PRESETS

From d6b16341d3d1cee2daafcfc17b7a14bac89fb098 Mon Sep 17 00:00:00 2001
From: Nicolai-vKuegelgen <nicolai.von-kuegelgen@bih-charite.de>
Date: Mon, 2 Sep 2024 11:49:37 +0200
Subject: [PATCH 6/8] Improve tests to cover effect of "--remote-dir-pattern"

---
 tests/test_sodar_ingest_fastq.py | 53 +++++++++++++++++++++++---------
 1 file changed, 38 insertions(+), 15 deletions(-)

diff --git a/tests/test_sodar_ingest_fastq.py b/tests/test_sodar_ingest_fastq.py
index 4fa1236d..2044869b 100644
--- a/tests/test_sodar_ingest_fastq.py
+++ b/tests/test_sodar_ingest_fastq.py
@@ -3,6 +3,7 @@
 We only run some smoke tests here.
 """
 
+import datetime
 import json
 import os
 import re
@@ -15,7 +16,12 @@
 
 from cubi_tk.__main__ import main, setup_argparse
 from cubi_tk.exceptions import ParameterException
-from cubi_tk.sodar.ingest_fastq import SodarIngestFastq
+from cubi_tk.irods_common import TransferJob
+from cubi_tk.sodar.ingest_fastq import (
+    DEST_PATTERN_PRESETS,
+    SRC_REGEX_PRESETS,
+    SodarIngestFastq,
+)
 
 from .conftest import my_get_sodar_info, my_sodar_api_export
 from .factories import InvestigationFactory
@@ -47,8 +53,6 @@ def test_run_sodar_ingest_fastq_nothing(capsys):
 
 
 def test_run_sodar_ingest_fastq_preset_definitions():
-    from cubi_tk.sodar.ingest_fastq import DEST_PATTERN_PRESETS, SRC_REGEX_PRESETS
-
     regexes = SRC_REGEX_PRESETS.keys()
     patterns = DEST_PATTERN_PRESETS.keys()
 
@@ -62,8 +66,6 @@ def test_run_sodar_ingest_fastq_preset_definitions():
 
 
 def test_run_sodar_ingest_fastq_default_preset_regex():
-    from cubi_tk.sodar.ingest_fastq import SRC_REGEX_PRESETS
-
     ## Test default regex
     # Collection of example filenames and the expected {sample} value the regex should capture
     test_filenames = {
@@ -78,8 +80,6 @@ def test_run_sodar_ingest_fastq_default_preset_regex():
 
 
 def test_run_sodar_ingest_fastq_digestiflow_preset_regex():
-    from cubi_tk.sodar.ingest_fastq import SRC_REGEX_PRESETS
-
     ## Test default regex
     # Collection of example filenames and the expected {sample} value the regex should capture
     pattern = "240101_XY01234_0000_B{flowcell}/A0000_{sample}/{flowcell}/{lane}/A0000_{sample}_S1_{lane}_R1_001.fastq.gz"
@@ -101,8 +101,6 @@ def test_run_sodar_ingest_fastq_digestiflow_preset_regex():
 
 
 def test_run_sodar_ingest_fastq_ont_preset_regex():
-    from cubi_tk.sodar.ingest_fastq import SRC_REGEX_PRESETS
-
     test_filenames = {
         "fake_base_path/20240101_A0000_sample1/20240101_0000_A1_AB12345_000xyz/bam_fail/"
         "AB12345_000xyz_pass_1c1234_0.bam": (
@@ -195,7 +193,6 @@ def test_run_sodar_ingest_fastq_smoke_test(mocker, requests_mock):
     # --- setup arguments
     irods_path = "/irods/dest"
     landing_zone_uuid = "466ab946-ce6a-4c78-9981-19b79e7bbe86"
-    # dest_path = "target/folder/"
     fake_base_path = "/base/path"
     argv = [
         "--verbose",
@@ -206,8 +203,6 @@ def test_run_sodar_ingest_fastq_smoke_test(mocker, requests_mock):
         "--sodar-api-token",
         "XXXX",
         "--yes",
-        # "--remote-dir-pattern",
-        # dest_path,
         fake_base_path,
         landing_zone_uuid,
     ]
@@ -223,16 +218,31 @@ def test_run_sodar_ingest_fastq_smoke_test(mocker, requests_mock):
 
     # --- add test files
     fake_file_paths = []
+    fake_dest_paths = []
+    date = datetime.date.today().strftime("%Y-%m-%d")
     for member in ("sample1", "sample2", "sample3"):
         for ext in ("", ".md5"):
             fake_file_paths.append(
                 "%s/%s/%s-N1-RNA1-RNA_seq1.fastq.gz%s" % (fake_base_path, member, member, ext)
             )
             fs.create_file(fake_file_paths[-1])
+            fake_dest_paths.append(
+                TransferJob(
+                    path_local=fake_file_paths[-1],
+                    path_remote=f"/irods/dest/{member}-N1-RNA1-RNA_seq1/raw_data/{date}/{member}-N1-RNA1-RNA_seq1.fastq.gz{ext}",
+                )
+            )
+
             fake_file_paths.append(
                 "%s/%s/%s-N1-DNA1-WES1.fq.gz%s" % (fake_base_path, member, member, ext)
             )
             fs.create_file(fake_file_paths[-1])
+            fake_dest_paths.append(
+                TransferJob(
+                    path_local=fake_file_paths[-1],
+                    path_remote=f"/irods/dest/{member}-N1-DNA1-WES1/raw_data/{date}/{member}-N1-DNA1-WES1.fq.gz{ext}",
+                )
+            )
 
     # Remove index's log MD5 file again so it is recreated.
     fs.remove(fake_file_paths[3])
@@ -298,8 +308,23 @@ def test_run_sodar_ingest_fastq_smoke_test(mocker, requests_mock):
     parser, _subparsers = setup_argparse()
     args = parser.parse_args(argv)
     ingestfastq = SodarIngestFastq(args)
+    assert ingestfastq.remote_dir_pattern == DEST_PATTERN_PRESETS["default"]
     lz, actual = ingestfastq.build_jobs()
-    assert len(actual) == len(fake_file_paths)
+    assert sorted(actual, key=lambda x: x.path_remote) == sorted(
+        fake_dest_paths, key=lambda x: x.path_remote
+    )
+
+    remote_pattern = "{collection_name}/target/folder/{filename}"
+    argv[-2:] = [
+        "--remote-dir-pattern",
+        remote_pattern,
+        fake_base_path,
+        landing_zone_uuid,
+    ]
+    parser, _subparsers = setup_argparse()
+    args = parser.parse_args(argv)
+    ingestfastq = SodarIngestFastq(args)
+    assert ingestfastq.remote_dir_pattern == remote_pattern
 
 
 def test_run_sodar_ingest_fastq_smoke_test_ont_preset(mocker, requests_mock):
@@ -319,8 +344,6 @@ def test_run_sodar_ingest_fastq_smoke_test_ont_preset(mocker, requests_mock):
         "--yes",
         "--preset",
         "ONT",
-        # "--remote-dir-pattern",
-        # dest_path,
         fake_base_path,
         landing_zone_uuid,
     ]

From 651380703ead80d35e03166f909e7ff29cad73c4 Mon Sep 17 00:00:00 2001
From: Nicolai-vKuegelgen <nicolai.von-kuegelgen@bih-charite.de>
Date: Mon, 2 Sep 2024 11:59:27 +0200
Subject: [PATCH 7/8] Also cover check remote path for ONT test

---
 tests/test_sodar_ingest_fastq.py | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/tests/test_sodar_ingest_fastq.py b/tests/test_sodar_ingest_fastq.py
index 2044869b..3fdc502a 100644
--- a/tests/test_sodar_ingest_fastq.py
+++ b/tests/test_sodar_ingest_fastq.py
@@ -331,7 +331,6 @@ def test_run_sodar_ingest_fastq_smoke_test_ont_preset(mocker, requests_mock):
     # --- setup arguments
     irods_path = "/irods/dest"
     landing_zone_uuid = "466ab946-ce6a-4c78-9981-19b79e7bbe86"
-    # dest_path = "target/folder/"
     fake_base_path = "/base/path"
     argv = [
         "--verbose",
@@ -361,6 +360,7 @@ def test_run_sodar_ingest_fastq_smoke_test_ont_preset(mocker, requests_mock):
     fake_file_paths = []
     date = "20240101"
     project_sample_id = "A0000_sample{n}"
+    fake_dest_paths = []
     for sample_n in (1, 2, 3):
         sample_path = project_sample_id.format(n=sample_n)
         # date _ time _ positions _ ID _ hash
@@ -378,6 +378,18 @@ def test_run_sodar_ingest_fastq_smoke_test_ont_preset(mocker, requests_mock):
             for ext in ("", ".md5"):
                 fake_file_paths.append(file_pattern + ext)
                 fs.create_file(fake_file_paths[-1])
+                # html files will NOT be recognised by the preset
+                if 'html' in fake_file_paths[-1]:
+                    continue
+                fake_dest_paths.append(
+                    TransferJob(
+                        path_local=fake_file_paths[-1],
+                        path_remote=os.path.join(
+                            f"/irods/dest/{sample_path}/raw_data/{flowcellrun}/",
+                            os.path.relpath(fake_file_paths[-1], f"{fake_base_path}/{date}_{sample_path}/{flowcellrun}"),
+                        )
+                    )
+                )
 
     # Remove MD5 file for sample 1 fail bam, so it is recreated.
     fs.remove(fake_file_paths[3])
@@ -444,4 +456,7 @@ def test_run_sodar_ingest_fastq_smoke_test_ont_preset(mocker, requests_mock):
     args = parser.parse_args(argv)
     ingestfastq = SodarIngestFastq(args)
     lz, actual = ingestfastq.build_jobs()
+    assert sorted(actual, key=lambda x: x.path_remote) == sorted(
+        fake_dest_paths, key=lambda x: x.path_remote
+    )
     assert len(actual) == len(fake_file_paths) - 6

From 82a1d386b3268eb1721c090b02ee15de528d3498 Mon Sep 17 00:00:00 2001
From: Nicolai-vKuegelgen <nicolai.von-kuegelgen@bih-charite.de>
Date: Mon, 2 Sep 2024 12:02:34 +0200
Subject: [PATCH 8/8] linting

---
 tests/test_sodar_ingest_fastq.py | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/tests/test_sodar_ingest_fastq.py b/tests/test_sodar_ingest_fastq.py
index 3fdc502a..118261db 100644
--- a/tests/test_sodar_ingest_fastq.py
+++ b/tests/test_sodar_ingest_fastq.py
@@ -379,15 +379,18 @@ def test_run_sodar_ingest_fastq_smoke_test_ont_preset(mocker, requests_mock):
                 fake_file_paths.append(file_pattern + ext)
                 fs.create_file(fake_file_paths[-1])
                 # html files will NOT be recognised by the preset
-                if 'html' in fake_file_paths[-1]:
+                if "html" in fake_file_paths[-1]:
                     continue
                 fake_dest_paths.append(
                     TransferJob(
                         path_local=fake_file_paths[-1],
                         path_remote=os.path.join(
                             f"/irods/dest/{sample_path}/raw_data/{flowcellrun}/",
-                            os.path.relpath(fake_file_paths[-1], f"{fake_base_path}/{date}_{sample_path}/{flowcellrun}"),
-                        )
+                            os.path.relpath(
+                                fake_file_paths[-1],
+                                f"{fake_base_path}/{date}_{sample_path}/{flowcellrun}",
+                            ),
+                        ),
                     )
                 )