52 pipeline should halt if bad t1ws are present or more generally if not bids valid (#54)

* added checks and error messages for too many dimension t1w's
* fixed logic for excluded subjects
* fixed bug where missing t1w's wouldn't halt pipeline
* format with black
* added validator check
* added tests for no t1w and invalid bids dir

A t1w file not found error would get caught by a try except that's now
been fixed by tracking all not found t1w errors and halting the pipeline
while alerting the user. Added full bids validator check using pip installable 
bids deno validator too.

Author: bendhouseart

data/participants.tsv

@@ -1,2 +1,2 @@
 participant_id	height	weight	age	gender
-sub-01	178	58	28	male
+sub-01	178	58	28	male

petdeface/petdeface.py

@@ -31,10 +31,12 @@
     from mideface import ApplyMideface
     from mideface import Mideface
     from pet import WeightedAverage
+    from utils import run_validator
 except ModuleNotFoundError:
     from .mideface import ApplyMideface
     from .mideface import Mideface
     from .pet import WeightedAverage
+    from .utils import run_validator
 
 
 # collect version from pyproject.toml
@@ -207,6 +209,10 @@ def deface(args: Union[dict, argparse.Namespace]) -> None:
     else:
         args = args
 
+    # first check to see if the dataset is bids valid
+    if not args.skip_bids_validator:
+        run_validator(args.bids_dir)
+
     if not check_valid_fs_license() and not locate_freesurfer_license().exists():
         raise Exception("You need a valid FreeSurfer license to proceed!")
 
@@ -255,6 +261,8 @@ def deface(args: Union[dict, argparse.Namespace]) -> None:
 
     petdeface_wf = Workflow(name="petdeface_wf", base_dir=output_dir)
 
+    missing_file_errors = []
+
     for subject_id in subjects:
         try:
             single_subject_wf = init_single_subject_wf(
@@ -265,12 +273,20 @@ def deface(args: Union[dict, argparse.Namespace]) -> None:
                 session_label=args.session_label,
                 session_label_exclude=args.session_label_exclude,
             )
-        except FileNotFoundError:
+        except FileNotFoundError as error:
             single_subject_wf = None
-
+            missing_file_errors.append(str(error))
         if single_subject_wf:
             petdeface_wf.add_nodes([single_subject_wf])
 
+    if (
+        missing_file_errors
+    ):  # todo add conditional later for cases where a template t1w is used
+        raise FileNotFoundError(
+            "The following subjects are missing t1w images:\n"
+            + "\n".join(missing_file_errors)
+        )
+
     try:
         petdeface_wf.write_graph("petdeface.dot", graph2use="colored", simple_form=True)
     except OSError as Err:
@@ -748,7 +764,7 @@ def __init__(
         anat_only=False,
         subject="",
         n_procs=2,
-        skip_bids_validator=True,
+        skip_bids_validator=False,
         remove_existing=True,
         placement="adjacent",
         preview_pics=True,

petdeface/utils.py

@@ -0,0 +1,50 @@
+import subprocess
+from pathlib import Path
+import json
+import sys
+
+
+class InvalidBIDSDataset(Exception):
+    def __init__(self, message, errors):
+        super().__init__(message)
+        self.errors = errors
+        print(f"{message}\n{errors}")
+
+
+def deno_validator_installed():
+    get_help = subprocess.run(
+        "bids-validator-deno --help", shell=True, capture_output=True
+    )
+    if get_help.returncode == 0:
+        return True
+    else:
+        return False
+
+
+def run_validator(bids_path):
+    bids_path = Path(bids_path)
+    if bids_path.exists():
+        pass
+    else:
+        raise FileNotFoundError(bids_path)
+    if deno_validator_installed():
+        command = f"bids-validator-deno {bids_path.resolve()} --ignoreWarnings --json --no-color"
+        run_validator = subprocess.run(command, shell=True, capture_output=True)
+        json_output = json.loads(run_validator.stdout.decode("utf-8"))
+        # since we've ignored warnings any issue in issue is an error
+        issues = json_output.get("issues").get("issues")
+        formatted_errors = ""
+        for issue in issues:
+            formatted_errors += "\n" + json.dumps(issue, indent=4)
+        if formatted_errors != "":
+            raise InvalidBIDSDataset(
+                message=f"Dataset at {bids_path} is invalid, see:",
+                errors=formatted_errors,
+            )
+
+    else:
+        raise Exception(
+            f"bids-validator-deno not found"
+            + "\nskip validation with --skip_bids_validator"
+            + "\nor install with pip install bids-validator-deno"
+        )

poetry.lock

@@ -1,6 +1,6 @@
 [tool.poetry]
 name = "petdeface"
-version = "0.2.2"
+version = "0.2.3"
 description = "A nipype PET and MR defacing pipeline for BIDS datasets utilizing FreeSurfer's MiDeFace."
 authors = ["Martin Nørgaard <[email protected]>", "Anthony Galassi <[email protected]>", "Murat Bilgel <[email protected]>"]
 license = "MIT"
@@ -19,6 +19,9 @@ python = ">=3.10, <4.0"
 setuptools = "^68.1.2"
 petutils = "^0.0.1"
 niworkflows = "^1.11.0"
+niftifixer = {git = "https://github.com/openneuropet/nifti_fixer.git"}
+bids-validator-deno = "^2.0.5"
+
 
 [tool.poetry.group.dev.dependencies]
 black = "^23.7.0"

pyproject.toml

@@ -3,6 +3,7 @@
 import shutil
 import bids
 from petdeface.petdeface import PetDeface
+from petdeface.utils import InvalidBIDSDataset
 from os import cpu_count
 from bids.layout import BIDSLayout
 import subprocess
@@ -34,6 +35,7 @@ def test_anat_in_first_session_folder():
             / "derivatives"
             / "petdeface",
             n_procs=nthreads,
+            preview_pics=False,
         )
         petdeface.run()
 
@@ -80,6 +82,7 @@ def test_anat_in_each_session_folder():
             / "derivatives"
             / "petdeface",
             n_procs=nthreads,
+            preview_pics=False,
         )
         petdeface.run()
 
@@ -117,5 +120,54 @@ def test_anat_in_subject_folder():
             / "derivatives"
             / "petdeface",
             n_procs=nthreads,
+            preview_pics=False,
         )
         petdeface.run()
+
+def test_no_anat():
+    # create a temporary directory to copy the existing dataset into
+    with tempfile.TemporaryDirectory() as tmpdir:
+        shutil.copytree(data_dir, Path(tmpdir) / "no_anat")
+
+        subject_folder = Path(tmpdir) / "no_anat" / "sub-01"
+        # next we delete the anat fold in the subject folder
+        shutil.rmtree(subject_folder / "ses-baseline" / "anat")
+
+        # run petdeface on the copied dataset
+        petdeface = PetDeface(
+            Path(tmpdir) / "no_anat",
+            output_dir=Path(tmpdir)
+            / "no_anat_defaced"
+            / "derivatives"
+            / "petdeface",
+            n_procs=nthreads,
+        )
+    
+        # now we want to assert that this pipeline crashes and print the error
+        with pytest.raises(FileNotFoundError) as exc_info:
+            petdeface.run()
+
+def test_invalid_bids():
+    with tempfile.TemporaryDirectory() as tmpdir:
+        shutil.copytree(data_dir, Path(tmpdir) / "invalid")
+        # rename the files in the pet folder to a different subject id
+        subject_folder = Path(tmpdir) / "invalid" / "sub-01"
+        pet_folder = subject_folder / "ses-baseline" / "pet"
+        for file in pet_folder.glob("sub-01_*"):
+            shutil.move(
+                file,
+                pet_folder / file.name.replace("sub-01", "sub-01-bestsubject")
+            )
+            
+        # run petdeface on the invalid dataset
+        petdeface = PetDeface(
+            Path(tmpdir) / "invalid",
+            output_dir=Path(tmpdir) / "invalid_defaced" / "derivatives" / "petdeface",
+            n_procs=nthreads,
+        )
+        
+        # Run it and see what error gets raised
+        with pytest.raises(InvalidBIDSDataset) as exc_info:
+            petdeface.run()
+        assert "Dataset at" in str(exc_info.value)
+