From 68455cb135881d3ab4d0d06a0c91e5735a2682ec Mon Sep 17 00:00:00 2001 From: Anthony Galassi <28850131+bendhouseart@users.noreply.github.com> Date: Wed, 16 Apr 2025 14:38:15 -0500 Subject: [PATCH 1/6] added checks and error messages for too many dimension t1w's --- petdeface/petdeface.py | 65 ++++++++++++++++++++++++++++++++++++++++++ pyproject.toml | 2 ++ 2 files changed, 67 insertions(+) diff --git a/petdeface/petdeface.py b/petdeface/petdeface.py index d29247a..14aa624 100755 --- a/petdeface/petdeface.py +++ b/petdeface/petdeface.py @@ -8,7 +8,11 @@ from bids import BIDSLayout import importlib import glob +import niftifixer from platform import system +import textwrap +import re + # import shutil import subprocess @@ -950,11 +954,72 @@ def main(): # noqa: max-complexity: 12 args.bids_dir = args.bids_dir.expanduser().resolve() else: args.bids_dir = args.bids_dir.absolute() + if isinstance(args.output_dir, pathlib.PosixPath) and "~" in str(args.output_dir): args.output_dir = args.output_dir.expanduser().resolve() else: if args.output_dir: args.output_dir = args.output_dir.absolute() + + # Sometimes there are extra dimensions in T1w images, typically the BIDS validator catches these, but + # they seem to get past the python of the version of the validator. What we do here is to perform a + # check on their dimesionality just incase. + check_these_t1ws = niftifixer.locate_t1ws(args.bids_dir) + bad_t1ws = [] + for t1w in check_these_t1ws: + nii_info = niftifixer.GetNiftiInfo(t1w) + if not nii_info.is_3D: + bad_t1ws.append(t1w) + if len(bad_t1ws) > 0: + # get terminal width + try: + width = os.get_terminal_size().columns + except OSError: + width = 80 + + # Regex to extract BIDS subject ID (text after sub- and before the next _) + # This pattern ensures we only get the subject ID, not the full path + subject_id_pattern = re.compile(r'sub-([^_/]+)') + + # Extract subject IDs from bad files for exclusion + bad_t1w_subject_ids = [] + for file in bad_t1ws: + match = subject_id_pattern.search(str(file)) + if match: + subject_id = match.group(1) + bad_t1w_subject_ids.append(subject_id) + bad_t1w_subject_ids = list(set(bad_t1w_subject_ids)) + + main_message = ( + f"Found {len(bad_t1ws)} t1w images with more than 3 dimensions. " + f"Reduce dimensionality to 3 for each T1w or exclude subjects with " + f"--participant_label_exclude. " + f"Or try using niftifixer before re-attempting; niftifixer is installed " + f"in the same environment as petdeface." + ) + + # Wrap the main message to fit the terminal width + wrapped_message = textwrap.fill(main_message, width=width) + + # print potentially helpful commands on their own lines + command_message = f"niftifixer {args.bids_dir}" + + if bad_t1w_subject_ids: + exclude_command = f"petdeface {args.bids_dir} {args.output_dir} --participant_label_exclude {' '.join(bad_t1w_subject_ids)}" + command_message += f"\n\nOr exclude problematic subjects:\n\n{exclude_command}" + + # Format the list of bad files + bad_files_message = "These files have too many dimensions ( >3 ):\n" + "\n".join(f" {file}" for file in bad_t1ws) + + # Combine all parts of the message + full_message = f"{wrapped_message}\n\n{command_message}\n\n{bad_files_message}" + + # Check if all problematic subjects are already in the exclude list + bad_tw1s_excluded = [t1w in args.participant_label_exclude for t1w in bad_t1w_subject_ids ] + if False in bad_t1w_subject_ids: + raise Exception(full_message) + else: + print(bad_files_message) if args.docker: check_docker_installed() diff --git a/pyproject.toml b/pyproject.toml index f473123..a742820 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -19,6 +19,8 @@ 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"} + [tool.poetry.group.dev.dependencies] black = "^23.7.0" From c72793502d4730d53ffab0b737da9c6bb5026d8f Mon Sep 17 00:00:00 2001 From: Anthony Galassi <28850131+bendhouseart@users.noreply.github.com> Date: Wed, 16 Apr 2025 14:48:43 -0500 Subject: [PATCH 2/6] fixed logic for excluded subjects --- petdeface/petdeface.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/petdeface/petdeface.py b/petdeface/petdeface.py index 14aa624..924cf06 100755 --- a/petdeface/petdeface.py +++ b/petdeface/petdeface.py @@ -1015,8 +1015,9 @@ def main(): # noqa: max-complexity: 12 full_message = f"{wrapped_message}\n\n{command_message}\n\n{bad_files_message}" # Check if all problematic subjects are already in the exclude list - bad_tw1s_excluded = [t1w in args.participant_label_exclude for t1w in bad_t1w_subject_ids ] - if False in bad_t1w_subject_ids: + bad_t1ws_excluded = [bad in args.participant_label_exclude for bad in bad_t1w_subject_ids ] + + if False in bad_t1ws_excluded: raise Exception(full_message) else: print(bad_files_message) From 5382ad13d675f28c3f739c5663e20ac5289f29bb Mon Sep 17 00:00:00 2001 From: Anthony Galassi <28850131+bendhouseart@users.noreply.github.com> Date: Thu, 22 May 2025 17:13:23 -0500 Subject: [PATCH 3/6] fixed bug where missing t1w's wouldn't halt pipeline 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. --- petdeface/petdeface.py | 78 ++++++------------------------------------ 1 file changed, 10 insertions(+), 68 deletions(-) diff --git a/petdeface/petdeface.py b/petdeface/petdeface.py index 924cf06..d1cc386 100755 --- a/petdeface/petdeface.py +++ b/petdeface/petdeface.py @@ -8,11 +8,7 @@ from bids import BIDSLayout import importlib import glob -import niftifixer from platform import system -import textwrap -import re - # import shutil import subprocess @@ -257,8 +253,11 @@ def deface(args: Union[dict, argparse.Namespace]) -> None: else: output_dir = args.output_dir + 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( @@ -269,12 +268,17 @@ 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: @@ -954,73 +958,11 @@ def main(): # noqa: max-complexity: 12 args.bids_dir = args.bids_dir.expanduser().resolve() else: args.bids_dir = args.bids_dir.absolute() - if isinstance(args.output_dir, pathlib.PosixPath) and "~" in str(args.output_dir): args.output_dir = args.output_dir.expanduser().resolve() else: if args.output_dir: args.output_dir = args.output_dir.absolute() - - # Sometimes there are extra dimensions in T1w images, typically the BIDS validator catches these, but - # they seem to get past the python of the version of the validator. What we do here is to perform a - # check on their dimesionality just incase. - check_these_t1ws = niftifixer.locate_t1ws(args.bids_dir) - bad_t1ws = [] - for t1w in check_these_t1ws: - nii_info = niftifixer.GetNiftiInfo(t1w) - if not nii_info.is_3D: - bad_t1ws.append(t1w) - if len(bad_t1ws) > 0: - # get terminal width - try: - width = os.get_terminal_size().columns - except OSError: - width = 80 - - # Regex to extract BIDS subject ID (text after sub- and before the next _) - # This pattern ensures we only get the subject ID, not the full path - subject_id_pattern = re.compile(r'sub-([^_/]+)') - - # Extract subject IDs from bad files for exclusion - bad_t1w_subject_ids = [] - for file in bad_t1ws: - match = subject_id_pattern.search(str(file)) - if match: - subject_id = match.group(1) - bad_t1w_subject_ids.append(subject_id) - bad_t1w_subject_ids = list(set(bad_t1w_subject_ids)) - - main_message = ( - f"Found {len(bad_t1ws)} t1w images with more than 3 dimensions. " - f"Reduce dimensionality to 3 for each T1w or exclude subjects with " - f"--participant_label_exclude. " - f"Or try using niftifixer before re-attempting; niftifixer is installed " - f"in the same environment as petdeface." - ) - - # Wrap the main message to fit the terminal width - wrapped_message = textwrap.fill(main_message, width=width) - - # print potentially helpful commands on their own lines - command_message = f"niftifixer {args.bids_dir}" - - if bad_t1w_subject_ids: - exclude_command = f"petdeface {args.bids_dir} {args.output_dir} --participant_label_exclude {' '.join(bad_t1w_subject_ids)}" - command_message += f"\n\nOr exclude problematic subjects:\n\n{exclude_command}" - - # Format the list of bad files - bad_files_message = "These files have too many dimensions ( >3 ):\n" + "\n".join(f" {file}" for file in bad_t1ws) - - # Combine all parts of the message - full_message = f"{wrapped_message}\n\n{command_message}\n\n{bad_files_message}" - - # Check if all problematic subjects are already in the exclude list - bad_t1ws_excluded = [bad in args.participant_label_exclude for bad in bad_t1w_subject_ids ] - - if False in bad_t1ws_excluded: - raise Exception(full_message) - else: - print(bad_files_message) if args.docker: check_docker_installed() From 3744c1784b700805d97d4416509715732662c742 Mon Sep 17 00:00:00 2001 From: Anthony Galassi <28850131+bendhouseart@users.noreply.github.com> Date: Thu, 22 May 2025 17:15:06 -0500 Subject: [PATCH 4/6] format with black --- petdeface/petdeface.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/petdeface/petdeface.py b/petdeface/petdeface.py index d1cc386..b8f4ede 100755 --- a/petdeface/petdeface.py +++ b/petdeface/petdeface.py @@ -253,7 +253,6 @@ def deface(args: Union[dict, argparse.Namespace]) -> None: else: output_dir = args.output_dir - petdeface_wf = Workflow(name="petdeface_wf", base_dir=output_dir) missing_file_errors = [] @@ -274,9 +273,12 @@ def deface(args: Union[dict, argparse.Namespace]) -> None: 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 + 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) + "The following subjects are missing t1w images:\n" + + "\n".join(missing_file_errors) ) try: From f83773aeddc01d581510dea39cdead07053862ec Mon Sep 17 00:00:00 2001 From: Anthony Galassi <28850131+bendhouseart@users.noreply.github.com> Date: Thu, 22 May 2025 18:13:28 -0500 Subject: [PATCH 5/6] added validator check --- petdeface/petdeface.py | 6 +++++ petdeface/utils.py | 50 ++++++++++++++++++++++++++++++++++++++++++ poetry.lock | 35 ++++++++++++++++++++++++++++- pyproject.toml | 3 ++- 4 files changed, 92 insertions(+), 2 deletions(-) create mode 100644 petdeface/utils.py diff --git a/petdeface/petdeface.py b/petdeface/petdeface.py index b8f4ede..647c01e 100755 --- a/petdeface/petdeface.py +++ b/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!") diff --git a/petdeface/utils.py b/petdeface/utils.py new file mode 100644 index 0000000..f176e83 --- /dev/null +++ b/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" + ) diff --git a/poetry.lock b/poetry.lock index a92396a..d5251b5 100644 --- a/poetry.lock +++ b/poetry.lock @@ -83,6 +83,21 @@ files = [ [package.dependencies] bidsschematools = ">=0.10" +[[package]] +name = "bids-validator-deno" +version = "2.0.5" +description = "Typescript implementation of the BIDS validator" +optional = false +python-versions = "*" +files = [ + {file = "bids_validator_deno-2.0.5-py2.py3-none-macosx_10_12_x86_64.whl", hash = "sha256:e7b0cd652a92c730ae00ad21e04ee430bc9cddad0bcdf5695a5fa47f17293568"}, + {file = "bids_validator_deno-2.0.5-py2.py3-none-macosx_11_0_arm64.whl", hash = "sha256:4bc4c8f7d957900df30d7e10f7e07d790925a1a522fcf673b883bafd82e32981"}, + {file = "bids_validator_deno-2.0.5-py2.py3-none-manylinux_2_17_aarch64.manylinux2014_aarch64.manylinux_2_24_aarch64.whl", hash = "sha256:7b676a27303d4d42b886d448964c1ec2502b1692ba440d51043b20e43778dc13"}, + {file = "bids_validator_deno-2.0.5-py2.py3-none-manylinux_2_24_x86_64.whl", hash = "sha256:db02e9bd2e7d58967b19a06a87961d4b0a1a41f450289c730f95eb7d184fe790"}, + {file = "bids_validator_deno-2.0.5-py2.py3-none-win_amd64.whl", hash = "sha256:9462c07aeeeaf9eb07777c14689f1c6cbf2ee7075c825ffde801e3c6d3a5b7e3"}, + {file = "bids_validator_deno-2.0.5.tar.gz", hash = "sha256:7316a373c0b326f1b9ec541aaeded5f6dc6e77631940ea08c2ce6fdc590e59ae"}, +] + [[package]] name = "bidsschematools" version = "0.11.3.post3" @@ -1449,6 +1464,24 @@ test = ["coverage (>=7.2)", "pytest", "pytest-cov", "pytest-doctestplus", "pytes typing = ["tox"] zstd = ["pyzstd (>=0.14.3)"] +[[package]] +name = "niftifixer" +version = "0.0.0" +description = "Fixes Nifti's" +optional = false +python-versions = ">=3.10, <4.0" +files = [] +develop = false + +[package.dependencies] +nibabel = "^5.2.1" + +[package.source] +type = "git" +url = "https://github.com/openneuropet/nifti_fixer.git" +reference = "HEAD" +resolved_reference = "a0469bb1ad2ff3d63c12ea536657d4fd9ab76b2c" + [[package]] name = "nilearn" version = "0.10.4" @@ -3228,4 +3261,4 @@ files = [ [metadata] lock-version = "2.0" python-versions = ">=3.10, <4.0" -content-hash = "d13eaeabc8d858d5540db6627dec495e106ff940245e5c50b9b2997e6b163b93" +content-hash = "6de8a7cc6be569cb4483d6784cb54ac7a6ca8cc97ccbb1dee8ee18f41895bd43" diff --git a/pyproject.toml b/pyproject.toml index a742820..3dfdec5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -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 ", "Anthony Galassi <28850131+bendhouseart@users.noreply.github.com>", "Murat Bilgel "] license = "MIT" @@ -20,6 +20,7 @@ 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] From 3bce864df178e21a317e7c93477520f0ae0df975 Mon Sep 17 00:00:00 2001 From: Anthony Galassi <28850131+bendhouseart@users.noreply.github.com> Date: Wed, 28 May 2025 09:09:28 -0500 Subject: [PATCH 6/6] added tests for no t1w and invalid bids dir --- data/participants.tsv | 2 +- petdeface/petdeface.py | 2 +- tests/test_dir_layouts.py | 52 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 2 deletions(-) diff --git a/data/participants.tsv b/data/participants.tsv index 2d076e5..8fd9e09 100644 --- a/data/participants.tsv +++ b/data/participants.tsv @@ -1,2 +1,2 @@ participant_id height weight age gender -sub-01 178 58 28 male \ No newline at end of file +sub-01 178 58 28 male diff --git a/petdeface/petdeface.py b/petdeface/petdeface.py index 647c01e..341b70b 100755 --- a/petdeface/petdeface.py +++ b/petdeface/petdeface.py @@ -764,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, diff --git a/tests/test_dir_layouts.py b/tests/test_dir_layouts.py index 167253f..60db604 100644 --- a/tests/test_dir_layouts.py +++ b/tests/test_dir_layouts.py @@ -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) + \ No newline at end of file