Conversation
There was a problem hiding this comment.
Pull request overview
Adds batch-processing support to sitk-cli commands, including detection of sitk.Image / sitk.Transform parameters to iterate over directories and generate per-file outputs.
Changes:
- Introduces batch wrapper logic to glob directories and match inputs by filename stem.
- Refactors CLI signature construction into new
introspectionandparametersmodules. - Updates public API/export surface (
create_command) and adds tests + docs for batch mode.
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
src/sitk_cli/batch.py |
Implements directory globbing + stem-matching batch execution wrapper. |
src/sitk_cli/lib.py |
Adds create_command(..., batch=True) and wires batch wrapper; deprecates make_cli. |
src/sitk_cli/parameters.py |
Centralizes building Typer-friendly signatures and translating SITK params to Path. |
src/sitk_cli/introspection.py |
Adds type-hint resolution helpers for detecting SITK param types. |
src/sitk_cli/constants.py |
Adds shared constant defaults for parameter names and glob patterns. |
tests/test_make_cli.py |
Adds tests that exercise new batch mode behavior. |
tests/test_introspection.py |
Adds tests for new introspection utilities and edge cases. |
README.md |
Documents batch mode usage and features. |
.github/workflows/deploy_and_test.yml |
Expands CI matrix and adjusts mypy invocation. |
pyproject.toml |
Packages typing marker file (py.typed) for downstream type checking. |
src/sitk_cli/__init__.py |
Exports create_command and bumps version. |
examples/batch_processing.py |
Adds end-to-end examples for batch workflows. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 20 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Match files by stem | ||
| if dir_files: | ||
| stem_to_files: dict[str, dict[str, Path]] = defaultdict(dict) | ||
| for pname, files in dir_files.items(): | ||
| for fpath in files: | ||
| stem, _ = get_stem_and_suffix(fpath) | ||
| stem_to_files[stem][pname] = fpath | ||
|
|
||
| # Filter complete matches | ||
| required_dir_params = {p for p in dir_files if p not in optional_params} | ||
| complete_matches = { | ||
| stem: files | ||
| for stem, files in stem_to_files.items() | ||
| if all(p in files for p in required_dir_params) | ||
| } | ||
|
|
||
| if not complete_matches: | ||
| msg = "No matching files found across all input directories" | ||
| raise FileNotFoundError(msg) | ||
|
|
||
| # Create output dir if needed | ||
| if has_output and out_dir: | ||
| out_dir.mkdir(parents=True, exist_ok=True) | ||
|
|
||
| # Process each match | ||
| total = len(complete_matches) | ||
| for idx, (stem, matched) in enumerate(complete_matches.items(), 1): | ||
| all_files = {**single_files, **matched} |
There was a problem hiding this comment.
complete_matches is only defined inside if dir_files: but is used unconditionally afterwards, which will raise UnboundLocalError when batch inputs are only single files (no directory parameters). Initialize complete_matches for the no-directory case (e.g., a single “match” run), and ensure the output naming has a sensible stem in that scenario (likely derived from output_stem’s file path).
| # Build argument map from args | ||
| param_names = list(func_sig.parameters) | ||
| arg_map: dict[str, Path] = {} | ||
| for i, arg in enumerate(args): | ||
| if i < len(param_names): | ||
| arg_map[param_names[i]] = arg | ||
|
|
There was a problem hiding this comment.
Positional argument mapping uses func_sig.parameters (original function), but args are for the batch wrapper signature (which may include output_dir and potentially different ordering). This will mis-assign positional values when users call batch mode positionally (e.g., cmd(input_dir, output_dir)), causing output_dir to be treated as an input path. Map positional args using the batch wrapper parameters (e.g., batch_sig.parameters) and ignore output_dir/output_template when constructing arg_map.
| # Check for overwrite protection | ||
| if overwrite is not True and not force and output_file.exists(): | ||
| if overwrite == "prompt": | ||
| # Interactive prompt | ||
| response = input(f"Output file '{output_file}' exists. Overwrite? [y/N]: ") | ||
| if response.lower() not in ("y", "yes"): | ||
| print("Operation cancelled.") | ||
| return | ||
| else: # overwrite is False | ||
| msg = ( | ||
| f"Output file '{output_file}' already exists. Use --force to overwrite." | ||
| ) | ||
| raise FileExistsError(msg) |
There was a problem hiding this comment.
In "prompt" mode, declining overwrite returns from _save_sitk_file() without signaling failure to the caller. As a result, the CLI wrapper can continue as if the operation succeeded (and may still return the computed object), which is inconsistent with “cancel” semantics. Consider raising typer.Abort (or returning a boolean indicating whether a save occurred and having the wrapper return None on cancel) to make cancellation explicit and consistent.
| **Features:** | ||
|
|
||
| - 🔍 Auto-detects files vs directories (globs `*.nii.gz` or `*.tfm`) | ||
| - 🔗 Matches files across directories by stem (e.g., `brain_001.nii.gz` ↔ `brain_001_mask.nii.gz`) |
There was a problem hiding this comment.
The example brain_001.nii.gz ↔ brain_001_mask.nii.gz does not share the same stem and won’t match with the current “exact stem equality” logic in batch.py. Update the example to reflect exact-stem matching (or document the actual matching rule if a different behavior is intended).
| - 🔗 Matches files across directories by stem (e.g., `brain_001.nii.gz` ↔ `brain_001_mask.nii.gz`) | |
| - 🔗 Matches files across directories by exact stem (e.g., `inputs/brain_001.nii.gz` ↔ `labels/brain_001.nii.gz`) |
1 participant
What do these changes do?
Adds support for batch processing of cli commands with detection of sitk.Image and sitk.Transform arguments is potential input to iterate (using glob pattern).
Related issue/s
Solves #16