Skip to content

Commit 4659cd6

Browse files
authored
[FIX] exit MATLAB / Octave on failure when using the python CLI (#1277)
* use exit code * bump submodules * exit on unexpected fails * test on several python * rm backslash in f strings * fix find action json * start implementing sub commands
1 parent 13dd553 commit 4659cd6

22 files changed

+416
-274
lines changed

.github/workflows/run_tests_cli.yml

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,11 @@ jobs:
1818
# only trigger update on upstream repo
1919
if: github.repository_owner == 'cpp-lln-lab'
2020

21+
strategy:
22+
fail-fast: false
23+
matrix:
24+
python-version: ['3.8', '3.9', '3.10', '3.11', '3.12']
25+
2126
steps:
2227

2328
- name: Install dependencies
@@ -34,8 +39,9 @@ jobs:
3439
node-version: 18
3540

3641
- uses: actions/setup-python@v5
42+
name: Set up Python ${{ matrix.python-version }}
3743
with:
38-
python-version: '3.10'
44+
python-version: ${{ matrix.python-version }}
3945

4046
- name: Clone bidspm
4147
uses: actions/checkout@v4
@@ -46,7 +52,7 @@ jobs:
4652
- name: Install validators
4753
run: |
4854
make install
49-
pip install .[dev]
55+
pip install .[test]
5056
5157
- name: Run tests and generate coverage report
5258
run: |
@@ -61,4 +67,3 @@ jobs:
6167
flags: cli
6268
name: codecov-cli
6369
fail_ci_if_error: false
64-
# token: ${{ secrets.CODECOV_TOKEN }} # not required but might help API rate limits
File renamed without changes.

bidspm.m

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ function displayArguments(varargin)
112112
end
113113

114114
function value = rootDir()
115-
value = fullfile(fileparts(mfilename('fullpath')));
115+
value = fileparts(mfilename('fullpath'));
116116
end
117117

118118
%% low level actions

demos/openneuro/ds002799_run.m

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
bidspm();
1313

1414
% The directory where the data are located
15-
root_dir = fullfile(fileparts(mfilename('fullpath')));
15+
root_dir = fileparts(mfilename('fullpath'));
1616
root_dir = pwd;
1717

1818
%% Parameters

demos/scripts/inverse_normalize.m

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
% (C) Copyright 2022 bidspm developers
22

3-
this_dir = fullfile(fileparts(mfilename('fullpath')));
3+
this_dir = fileparts(mfilename('fullpath'));
44

55
% we are using the output data from the MoAE demo
66
% so make sure you have run it before trying this.

lib/spm_2_bids

pyproject.toml

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,8 @@ demo = [
3030
"octave-kernel"
3131
]
3232
dev = [
33-
"bidspm[demo,doc,style]",
33+
"bidspm[demo,doc,style,test]",
3434
"cffconvert",
35-
"coverage",
36-
"pytest",
3735
"ruamel.yaml",
3836
'pandas'
3937
]
@@ -53,20 +51,17 @@ doc = [
5351
]
5452
docs = ["bidspm[doc]"]
5553
style = [
56-
"black",
57-
"codespell",
58-
"flake8",
59-
"flake8-docstrings",
60-
"miss-hit",
61-
"mypy",
6254
"pre-commit",
63-
"reorder-python-imports",
64-
"rstcheck",
6555
"sourcery"
6656
]
57+
test = [
58+
"coverage",
59+
"pytest"
60+
]
6761

6862
[project.scripts]
69-
bidspm = "bidspm.bidspm:cli"
63+
bidspm = "bidspm.cli:cli"
64+
bidspm_2 = "bidspm.cli:new_cli"
7065
validate_model = "bidspm.validate:cli"
7166

7267
[project.urls]

src/bidspm/bidspm.py

Lines changed: 13 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -2,23 +2,21 @@
22
from __future__ import annotations
33

44
import subprocess
5-
import sys
65
from pathlib import Path
76
from typing import Any
87

98
from rich import print
109

1110
from .matlab import matlab
12-
from .parsers import bidspm_log, common_parser
11+
from .parsers import bidspm_log
1312

1413
log = bidspm_log(name="bidspm")
1514

16-
new_line = ", ...\n\t "
15+
new_line = ", ...\n\t "
1716

1817

1918
def base_cmd(bids_dir: Path, output_dir: Path) -> str:
20-
cmd = " bidspm('init');"
21-
cmd += f" bidspm( '{bids_dir}'{new_line}'{output_dir}'"
19+
cmd = f" bidspm( '{bids_dir}'{new_line}'{output_dir}'"
2220
return cmd
2321

2422

@@ -28,7 +26,7 @@ def append_main_cmd(cmd: str, analysis_level: str, action: str) -> str:
2826

2927

3028
def end_cmd(cmd: str) -> str:
31-
cmd += "); exit;"
29+
cmd += "); exit;"
3230
return cmd
3331

3432

@@ -99,10 +97,6 @@ def default_model(
9997
ignore: list[str] | None = None,
10098
options: Path | None = None,
10199
) -> int | str:
102-
if space and len(space) > 1:
103-
log.error(f"Only one space allowed for statistical analysis. Got\n:{space}")
104-
return 1
105-
106100
if task is None:
107101
task = "{''}"
108102

@@ -141,10 +135,6 @@ def preprocess(
141135
dry_run: bool = False,
142136
options: Path | None = None,
143137
) -> int | str:
144-
if action == "preprocess" and task and len(task) > 1:
145-
log.error(f"Only one task allowed for preprocessing. Got\n:{task}")
146-
return 1
147-
148138
cmd = generate_cmd(
149139
bids_dir=bids_dir,
150140
output_dir=output_dir,
@@ -229,8 +219,8 @@ def stats(
229219
output_dir: Path,
230220
analysis_level: str,
231221
action: str,
232-
preproc_dir: Path,
233-
model_file: Path,
222+
preproc_dir: Path | None,
223+
model_file: Path | None,
234224
verbosity: int = 2,
235225
participant_label: list[str] | None = None,
236226
fwhm: Any = 6,
@@ -246,10 +236,6 @@ def stats(
246236
keep_residuals: bool = False,
247237
options: Path | None = None,
248238
) -> int | str:
249-
if space and len(space) > 1:
250-
log.error(f"Only one space allowed for statistical analysis. Got\n:{space}")
251-
return 1
252-
253239
cmd = generate_cmd(
254240
bids_dir=bids_dir,
255241
output_dir=output_dir,
@@ -281,7 +267,7 @@ def stats(
281267
cmd += f"{new_line}'keep_residuals', true"
282268
cmd = end_cmd(cmd)
283269

284-
log.info("Running statistics.")
270+
log.info(f"Running {action}.")
285271

286272
return cmd
287273

@@ -310,63 +296,6 @@ def generate_cmd(
310296
return cmd
311297

312298

313-
def cli(argv: Any = sys.argv) -> None:
314-
parser = common_parser()
315-
316-
args, _ = parser.parse_known_args(argv[1:])
317-
318-
bids_dir = Path(args.bids_dir[0]).absolute()
319-
output_dir = Path(args.output_dir[0]).absolute()
320-
analysis_level = args.analysis_level[0]
321-
action = args.action[0]
322-
roi_atlas = args.roi_atlas[0]
323-
bids_filter_file = (
324-
Path(args.bids_filter_file[0]).absolute()
325-
if args.bids_filter_file is not None
326-
else None
327-
)
328-
preproc_dir = (
329-
Path(args.preproc_dir[0]).absolute() if args.preproc_dir is not None else None
330-
)
331-
model_file = (
332-
Path(args.model_file[0]).absolute() if args.model_file is not None else None
333-
)
334-
options = Path(args.options).absolute() if args.options is not None else None
335-
336-
return_code = bidspm(
337-
bids_dir,
338-
output_dir,
339-
analysis_level,
340-
action=action,
341-
participant_label=args.participant_label,
342-
verbosity=args.verbosity,
343-
task=args.task,
344-
space=args.space,
345-
ignore=args.ignore,
346-
fwhm=args.fwhm,
347-
bids_filter_file=bids_filter_file,
348-
dummy_scans=args.dummy_scans,
349-
anat_only=args.anat_only,
350-
skip_validation=args.skip_validation,
351-
dry_run=args.dry_run,
352-
preproc_dir=preproc_dir,
353-
model_file=model_file,
354-
roi_based=args.roi_based,
355-
roi_atlas=roi_atlas,
356-
roi_name=args.roi_name,
357-
roi_dir=args.roi_dir,
358-
concatenate=args.concatenate,
359-
design_only=args.design_only,
360-
keep_residuals=args.keep_residuals,
361-
options=options,
362-
)
363-
364-
if return_code == 1:
365-
sys.exit(1)
366-
else:
367-
sys.exit(0)
368-
369-
370299
def bidspm(
371300
bids_dir: Path,
372301
output_dir: Path,
@@ -394,13 +323,6 @@ def bidspm(
394323
keep_residuals: bool = False,
395324
options: Path | None = None,
396325
) -> int:
397-
if not bids_dir.is_dir():
398-
log.error(f"The 'bids_dir' does not exist:\n\t{bids_dir}")
399-
return 1
400-
401-
if preproc_dir is not None and not preproc_dir.is_dir():
402-
log.error(f"The 'preproc_dir' does not exist:\n\t{preproc_dir}")
403-
return 1
404326

405327
if action == "default_model":
406328
cmd = default_model(
@@ -448,14 +370,6 @@ def bidspm(
448370
)
449371

450372
elif action in {"stats", "contrasts", "results"}:
451-
if preproc_dir is None or not preproc_dir.exists():
452-
log.error(f"'preproc_dir' must be specified for stats. Got:\n{preproc_dir}")
453-
return 1
454-
455-
if model_file is None or not model_file.exists():
456-
log.error(f"'model_file' must be specified for stats. Got:\n{model_file}")
457-
return 1
458-
459373
cmd = stats(
460374
bids_dir=bids_dir,
461375
output_dir=output_dir,
@@ -477,11 +391,13 @@ def bidspm(
477391
keep_residuals=keep_residuals,
478392
options=options,
479393
)
480-
else:
481-
log.error(f"\nunknown action: {action}")
482-
return 1
483394

484-
return cmd if isinstance(cmd, int) else run_command(cmd)
395+
if isinstance(cmd, int):
396+
return cmd
397+
398+
cmd = f" bidspm('init'); try; {cmd} catch; exit; end;"
399+
400+
return run_command(cmd)
485401

486402

487403
def run_command(cmd: str, platform: str | None = None) -> int:

0 commit comments

Comments
 (0)