From f356ada7859774d7846fc8fd339d9fdeb4737ba9 Mon Sep 17 00:00:00 2001
From: Michelle Wang <tomichellewang@gmail.com>
Date: Sun, 14 Jul 2024 23:38:35 +0200
Subject: [PATCH] [FIX] Fix `nipoppy run` creating empty PyBIDS database on
 second (and later) loop iteration (#283)

* fix pybids ignore pattern bug

* update tests
---
 nipoppy_cli/nipoppy/workflows/pipeline.py   | 12 +++++----
 nipoppy_cli/tests/test_workflow_pipeline.py | 26 +++++++++++++++---
 nipoppy_cli/tests/test_workflow_runner.py   | 30 ++++++++++++++++++++-
 3 files changed, 59 insertions(+), 9 deletions(-)

diff --git a/nipoppy_cli/nipoppy/workflows/pipeline.py b/nipoppy_cli/nipoppy/workflows/pipeline.py
index 1b334abe..cb033bbd 100644
--- a/nipoppy_cli/nipoppy/workflows/pipeline.py
+++ b/nipoppy_cli/nipoppy/workflows/pipeline.py
@@ -270,20 +270,22 @@ def set_up_bids_db(
         """Set up the BIDS database."""
         dpath_bids_db: Path = Path(dpath_bids_db)
 
+        pybids_ignore_patterns = self.pybids_ignore_patterns.copy()
+
         if participant_id is not None:
             add_pybids_ignore_patterns(
-                current=self.pybids_ignore_patterns,
+                current=pybids_ignore_patterns,
                 new=f"^(?!/{BIDS_SUBJECT_PREFIX}({participant_id}))",
             )
         if session_id is not None:
             add_pybids_ignore_patterns(
-                current=self.pybids_ignore_patterns,
+                current=pybids_ignore_patterns,
                 new=f".*?/{BIDS_SESSION_PREFIX}(?!{session_id})",
             )
 
         self.logger.info(
-            f"Building BIDSLayout with {len(self.pybids_ignore_patterns)} ignore "
-            f"patterns: {self.pybids_ignore_patterns}"
+            f"Building BIDSLayout with {len(pybids_ignore_patterns)} ignore "
+            f"patterns: {pybids_ignore_patterns}"
         )
 
         if dpath_bids_db.exists() and list(dpath_bids_db.iterdir()):
@@ -295,7 +297,7 @@ def set_up_bids_db(
         bids_layout: bids.BIDSLayout = create_bids_db(
             dpath_bids=self.layout.dpath_bids,
             dpath_bids_db=dpath_bids_db,
-            ignore_patterns=self.pybids_ignore_patterns,
+            ignore_patterns=pybids_ignore_patterns,
             reset_database=True,
         )
 
diff --git a/nipoppy_cli/tests/test_workflow_pipeline.py b/nipoppy_cli/tests/test_workflow_pipeline.py
index 5f0535df..aca63dfc 100644
--- a/nipoppy_cli/tests/test_workflow_pipeline.py
+++ b/nipoppy_cli/tests/test_workflow_pipeline.py
@@ -55,11 +55,11 @@ def get_participants_sessions_to_run(
             participant_id=participant_id, session_id=session_id
         )
 
-    def run_single(self, subject: str, session_id: str):
+    def run_single(self, participant: str, session_id: str):
         """Run on a single participant_id/session_id."""
         self._n_runs += 1
-        self.logger.info(f"Running on {subject}/{session_id}")
-        if subject == "FAIL":
+        self.logger.info(f"Running on participant {participant}, session {session_id}")
+        if participant == "FAIL":
             self._n_errors += 1
             raise RuntimeError("FAIL")
 
@@ -498,6 +498,26 @@ def test_set_up_bids_db(
     assert len(bids_layout.get(extension=".nii.gz")) == expected_count
 
 
+def test_set_up_bids_db_ignore_patterns(workflow: PipelineWorkflow, tmp_path: Path):
+    dpath_bids_db = tmp_path / "bids_db"
+    participant_id = "01"
+    session_id = "1"
+
+    fids.create_fake_bids_dataset(
+        output_dir=workflow.layout.dpath_bids,
+    )
+
+    pybids_ignore_patterns = workflow.pybids_ignore_patterns[:]
+
+    workflow.set_up_bids_db(
+        dpath_bids_db=dpath_bids_db,
+        participant_id=participant_id,
+        session_id=session_id,
+    )
+
+    assert pybids_ignore_patterns == workflow.pybids_ignore_patterns
+
+
 @pytest.mark.parametrize(
     "pipeline_name,expected_version",
     [("heudiconv", "0.12.2"), ("fmriprep", "23.1.3"), ("my_pipeline", "1.0")],
diff --git a/nipoppy_cli/tests/test_workflow_runner.py b/nipoppy_cli/tests/test_workflow_runner.py
index 47654cd7..aa06bf7d 100644
--- a/nipoppy_cli/tests/test_workflow_runner.py
+++ b/nipoppy_cli/tests/test_workflow_runner.py
@@ -4,6 +4,7 @@
 from pathlib import Path
 
 import pytest
+from bids import BIDSLayout
 from fids import fids
 
 from nipoppy.config.main import Config
@@ -11,7 +12,7 @@
 from nipoppy.tabular.doughnut import Doughnut
 from nipoppy.workflows.runner import PipelineRunner
 
-from .conftest import create_empty_dataset, get_config
+from .conftest import create_empty_dataset, get_config, prepare_dataset
 
 
 @pytest.fixture(scope="function")
@@ -262,3 +263,30 @@ def test_get_participants_sessions_to_run(
             participant_id=participant_id, session_id=session_id
         )
     ] == expected
+
+
+def test_run_multiple(config: Config, tmp_path: Path):
+    participant_id = None
+    session_id = None
+    runner = PipelineRunner(
+        dpath_root=tmp_path,
+        pipeline_name="dummy_pipeline",
+        pipeline_version="1.0.0",
+        participant_id=participant_id,
+        session_id=session_id,
+    )
+    config.save(runner.layout.fpath_config)
+
+    participants_and_sessions = {"01": ["1"], "02": ["2"]}
+    create_empty_dataset(runner.layout.dpath_root)
+    manifest = prepare_dataset(
+        participants_and_sessions_manifest=participants_and_sessions,
+        participants_and_sessions_bidsified=participants_and_sessions,
+        dpath_bidsified=runner.layout.dpath_bids,
+    )
+    manifest.save_with_backup(runner.layout.fpath_manifest)
+    runner.run_setup()
+    runner.run_main()
+
+    bids_layout = BIDSLayout(database_path=runner.dpath_pipeline_bids_db)
+    assert not len(bids_layout.get(extension=".nii.gz")) == 0