From 12240946e100b7db94b3e3a81bc0ab1402aece9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fernando=20P=C3=A9rez-Garc=C3=ADa?= Date: Sun, 13 Apr 2025 22:11:53 +0100 Subject: [PATCH 1/5] Address code issues raised by qlty --- .../workflows/check_pull_request_title.yml | 5 ++ .github/workflows/code_quality.yml | 4 + .github/workflows/publish.yml | 3 + .github/workflows/tests.yml | 4 + src/torchio/utils.py | 80 ++++++++++++------- 5 files changed, 65 insertions(+), 31 deletions(-) diff --git a/.github/workflows/check_pull_request_title.yml b/.github/workflows/check_pull_request_title.yml index 061b74649..9ecfaf47f 100644 --- a/.github/workflows/check_pull_request_title.yml +++ b/.github/workflows/check_pull_request_title.yml @@ -1,8 +1,13 @@ name: 'Check PR title' + on: pull_request: types: [edited, opened, synchronize, reopened] +permissions: + pull-requests: read + statuses: write # Required to update commit status (e.g. pass/fail) + jobs: pr-title-check: runs-on: ubuntu-latest diff --git a/.github/workflows/code_quality.yml b/.github/workflows/code_quality.yml index 3e2350487..3afdbc00c 100644 --- a/.github/workflows/code_quality.yml +++ b/.github/workflows/code_quality.yml @@ -8,6 +8,10 @@ on: schedule: - cron: "0 4 * * *" +permissions: + contents: read + statuses: write + env: FORCE_COLOR: 1 diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 750e67319..4acfaa35a 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -2,6 +2,9 @@ name: Publish on: push +permissions: + pull-requests: read + jobs: build: name: Build the package diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 6571e4901..f080cd8ea 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -8,6 +8,10 @@ on: schedule: - cron: "0 4 * * *" +permissions: + contents: read + statuses: write + env: FORCE_COLOR: 1 diff --git a/src/torchio/utils.py b/src/torchio/utils.py index 1f926780e..19bd692ac 100644 --- a/src/torchio/utils.py +++ b/src/torchio/utils.py @@ -27,6 +27,9 @@ from .types import TypeNumber from .types import TypePath +ITK_SNAP = 'ITK-SNAP' +SLICER = 'Slicer' + def to_tuple( value: Any, @@ -344,46 +347,61 @@ def add_images_from_batch( def guess_external_viewer() -> Optional[Path]: - """Guess the path to an executable that could be used to visualize images. + """Guess the path to an executable that can be used to visualize images. - Currently, it looks for 1) ITK-SNAP and 2) 3D Slicer. Implemented - for macOS and Windows. + It looks for 1) ITK-SNAP and 2) 3D Slicer. """ if 'SITK_SHOW_COMMAND' in os.environ: return Path(os.environ['SITK_SHOW_COMMAND']) - platform = sys.platform - itk = 'ITK-SNAP' - slicer = 'Slicer' - if platform == 'darwin': + + if (platform := sys.platform) == 'darwin': + return _guess_macos_viewer() + elif platform == 'win32': + return _guess_windows_viewer() + elif 'linux' in platform: + return _guess_linux_viewer() + return None # for mypy + + +def _guess_macos_viewer() -> Optional[Path]: + def _get_app_path(app_name: str) -> Path: app_path = '/Applications/{}.app/Contents/MacOS/{}' - itk_snap_path = Path(app_path.format(2 * (itk,))) + return Path(app_path.format(2 * (app_name,))) + + if (itk_snap_path := _get_app_path(ITK_SNAP)).is_file(): + return itk_snap_path + + if (slicer_path := _get_app_path(SLICER)).is_file(): + return slicer_path + + +def _guess_windows_viewer() -> Optional[Path]: + def _get_app_path(app_dirs: list[Path], bin_name: str) -> Path: + app_dir = app_dirs[-1] + app_path = app_dir / bin_name + if app_path.is_file(): + return app_path + + program_files_dir = Path(os.environ['ProgramW6432']) + itk_snap_dirs = list(program_files_dir.glob(f'{ITK_SNAP}*')) + if itk_snap_dirs: + itk_snap_path = _get_app_path(itk_snap_dirs, 'bin/itk-snap.exe') if itk_snap_path.is_file(): return itk_snap_path - slicer_path = Path(app_path.format(2 * (slicer,))) + + slicer_dirs = list(program_files_dir.glob(f'{SLICER}*')) + if slicer_dirs: + slicer_path = _get_app_path(slicer_dirs, 'slicer.exe') if slicer_path.is_file(): return slicer_path - elif platform == 'win32': - program_files_dir = Path(os.environ['ProgramW6432']) - itk_snap_dirs = list(program_files_dir.glob('ITK-SNAP*')) - if itk_snap_dirs: - itk_snap_dir = itk_snap_dirs[-1] - itk_snap_path = itk_snap_dir / 'bin/itk-snap.exe' - if itk_snap_path.is_file(): - return itk_snap_path - slicer_dirs = list(program_files_dir.glob('Slicer*')) - if slicer_dirs: - slicer_dir = slicer_dirs[-1] - slicer_path = slicer_dir / 'slicer.exe' - if slicer_path.is_file(): - return slicer_path - elif 'linux' in platform: - itk_snap_which = shutil.which('itksnap') - if itk_snap_which is not None: - return Path(itk_snap_which) - slicer_which = shutil.which('Slicer') - if slicer_which is not None: - return Path(slicer_which) - return None # for mypy + + +def _guess_linux_viewer() -> Optional[Path]: + if (itk_snap_which := shutil.which('itksnap')) is not None: + return Path(itk_snap_which) + + if (slicer_which := shutil.which('Slicer')) is not None: + return Path(slicer_which) def parse_spatial_shape(shape): From ddd836f4aa01f5f0329c6f61b368da38b86d4e5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fernando=20P=C3=A9rez-Garc=C3=ADa?= Date: Sun, 13 Apr 2025 22:18:12 +0100 Subject: [PATCH 2/5] Fix regex --- .github/workflows/check_pull_request_title.yml | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/.github/workflows/check_pull_request_title.yml b/.github/workflows/check_pull_request_title.yml index 9ecfaf47f..0d6d4a49b 100644 --- a/.github/workflows/check_pull_request_title.yml +++ b/.github/workflows/check_pull_request_title.yml @@ -18,12 +18,22 @@ jobs: run: echo ${{ github.event.pull_request.user.login }} - uses: naveenk1223/action-pr-title@master with: - # Valid titles: "Do something" + # Valid titles: + # - "Do something" + # - "Address something" # Invalid title: # - "do something" # - "Do something." # - "Does something" # - "Do" - regex: '^[A-Z][a-zA-Z]*(? + ^ # Start of string + [A-Z] # First character must be an uppercase ASCII letter + [a-zA-Z]* # Followed by zero or more ASCII letters + (? Date: Sun, 13 Apr 2025 22:20:31 +0100 Subject: [PATCH 3/5] Fix regex --- .github/workflows/check_pull_request_title.yml | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/.github/workflows/check_pull_request_title.yml b/.github/workflows/check_pull_request_title.yml index 0d6d4a49b..6671ae124 100644 --- a/.github/workflows/check_pull_request_title.yml +++ b/.github/workflows/check_pull_request_title.yml @@ -18,6 +18,14 @@ jobs: run: echo ${{ github.event.pull_request.user.login }} - uses: naveenk1223/action-pr-title@master with: + # ^ Start of string + # [A-Z] First character must be an uppercase ASCII letter + # [a-zA-Z]* Followed by zero or more ASCII letters + # (? - ^ # Start of string - [A-Z] # First character must be an uppercase ASCII letter - [a-zA-Z]* # Followed by zero or more ASCII letters - (? Date: Sun, 13 Apr 2025 22:27:22 +0100 Subject: [PATCH 4/5] Fix typing issues --- src/torchio/datasets/ixi.py | 14 +++++++------- src/torchio/utils.py | 21 +++++++++++++-------- 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/src/torchio/datasets/ixi.py b/src/torchio/datasets/ixi.py index 53707d37c..be823250d 100644 --- a/src/torchio/datasets/ixi.py +++ b/src/torchio/datasets/ixi.py @@ -107,7 +107,7 @@ def _check_exists(root, modalities): return exists @staticmethod - def _get_subjects_list(root, modalities): + def _get_subjects_list(root: Path, modalities: Sequence[str]) -> list[Subject]: # The number of files for each modality is not the same # E.g. 581 for T1, 578 for T2 # Let's just use the first modality as reference for now @@ -137,7 +137,7 @@ def _get_subjects_list(root, modalities): subjects.append(Subject(**images_dict)) return subjects - def _download(self, root, modalities): + def _download(self, root: Path, modalities: Sequence[str]) -> None: """Download the IXI data if it does not exist already.""" for modality in modalities: modality_dir = root / modality @@ -195,7 +195,7 @@ def __init__( super().__init__(subjects_list, transform=transform, **kwargs) @staticmethod - def _get_subjects_list(root): + def _get_subjects_list(root: Path) -> list[Subject]: image_paths = sglob(root / 'image', '*.nii.gz') label_paths = sglob(root / 'label', '*.nii.gz') if not (image_paths and label_paths): @@ -214,7 +214,7 @@ def _get_subjects_list(root): subjects.append(Subject(**subject_dict)) return subjects - def _download(self, root): + def _download(self, root: Path) -> None: """Download the tiny IXI data if it doesn't exist already.""" if root.is_dir(): # assume it's been downloaded print('Root directory for IXITiny found:', root) # noqa: T201 @@ -234,9 +234,9 @@ def _download(self, root): shutil.rmtree(ixi_tiny_dir) -def sglob(directory, pattern): - return sorted(Path(directory).glob(pattern)) +def sglob(directory: Path, pattern: str) -> list[Path]: + return sorted(directory.glob(pattern)) -def get_subject_id(path): +def get_subject_id(path: Path) -> str: return '-'.join(path.name.split('-')[:-1]) diff --git a/src/torchio/utils.py b/src/torchio/utils.py index 19bd692ac..16ec0ecfc 100644 --- a/src/torchio/utils.py +++ b/src/torchio/utils.py @@ -360,7 +360,8 @@ def guess_external_viewer() -> Optional[Path]: return _guess_windows_viewer() elif 'linux' in platform: return _guess_linux_viewer() - return None # for mypy + else: + return None def _guess_macos_viewer() -> Optional[Path]: @@ -370,9 +371,10 @@ def _get_app_path(app_name: str) -> Path: if (itk_snap_path := _get_app_path(ITK_SNAP)).is_file(): return itk_snap_path - - if (slicer_path := _get_app_path(SLICER)).is_file(): + elif (slicer_path := _get_app_path(SLICER)).is_file(): return slicer_path + else: + return None def _guess_windows_viewer() -> Optional[Path]: @@ -384,24 +386,27 @@ def _get_app_path(app_dirs: list[Path], bin_name: str) -> Path: program_files_dir = Path(os.environ['ProgramW6432']) itk_snap_dirs = list(program_files_dir.glob(f'{ITK_SNAP}*')) + slicer_dirs = list(program_files_dir.glob(f'{SLICER}*')) + if itk_snap_dirs: itk_snap_path = _get_app_path(itk_snap_dirs, 'bin/itk-snap.exe') if itk_snap_path.is_file(): return itk_snap_path - - slicer_dirs = list(program_files_dir.glob(f'{SLICER}*')) - if slicer_dirs: + elif slicer_dirs: slicer_path = _get_app_path(slicer_dirs, 'slicer.exe') if slicer_path.is_file(): return slicer_path + else: + return None def _guess_linux_viewer() -> Optional[Path]: if (itk_snap_which := shutil.which('itksnap')) is not None: return Path(itk_snap_which) - - if (slicer_which := shutil.which('Slicer')) is not None: + elif (slicer_which := shutil.which('Slicer')) is not None: return Path(slicer_which) + else: + return None def parse_spatial_shape(shape): From 4d57d7ceed1826889e30464b5f9ea034ca8df8cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fernando=20P=C3=A9rez-Garc=C3=ADa?= Date: Tue, 13 May 2025 08:43:26 +0100 Subject: [PATCH 5/5] Remove Optional --- src/torchio/utils.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/torchio/utils.py b/src/torchio/utils.py index 02660efef..4a06e46d0 100644 --- a/src/torchio/utils.py +++ b/src/torchio/utils.py @@ -362,7 +362,7 @@ def guess_external_viewer() -> Path | None: return None -def _guess_macos_viewer() -> Optional[Path]: +def _guess_macos_viewer() -> Path | None: def _get_app_path(app_name: str) -> Path: app_path = '/Applications/{}.app/Contents/MacOS/{}' return Path(app_path.format(2 * (app_name,))) @@ -375,7 +375,7 @@ def _get_app_path(app_name: str) -> Path: return None -def _guess_windows_viewer() -> Optional[Path]: +def _guess_windows_viewer() -> Path | None: def _get_app_path(app_dirs: list[Path], bin_name: str) -> Path: app_dir = app_dirs[-1] app_path = app_dir / bin_name @@ -398,7 +398,7 @@ def _get_app_path(app_dirs: list[Path], bin_name: str) -> Path: return None -def _guess_linux_viewer() -> Optional[Path]: +def _guess_linux_viewer() -> Path | None: if (itk_snap_which := shutil.which('itksnap')) is not None: return Path(itk_snap_which) elif (slicer_which := shutil.which('Slicer')) is not None: