BUG: Fix several bugs (#839)

docs/source/v1.6.md.inc

@@ -5,6 +5,7 @@
 :new: New features & enhancements
 
 - Added [`regress_artifact`][mne_bids_pipeline._config.regress_artifact] to allow artifact regression (e.g., of MEG reference sensors in KIT systems) (#837 by @larsoner)
+- Chosen `reject` parameters are now saved in the generated HTML reports (#839 by @larsoner)
 
 [//]: # (### :warning: Behavior changes)
 
@@ -17,6 +18,9 @@
 ### :bug: Bug fixes
 
 - Fix minor issues with path handling for cross-talk and calibration files (#834 by @larsoner)
+- Fix bug where EEG `reject` params were not used for `ch_types = ["meg", "eeg"]` (#839 by @larsoner)
+- Fix bug where implicit `mf_reference_run` could change across invocations of `mne_bids_pipeline`, breaking caching (#839 by @larsoner)
+- Fix bug where `--no-cache` had no effect (#839 by @larsoner)
 
 ### :medical_symbol: Code health
 

mne_bids_pipeline/_config_import.py

@@ -369,7 +369,7 @@ def _pydantic_validate(
     if config_path is not None:
         name += f" from {config_path}"
     model_config = ConfigDict(
-        arbitrary_types_allowed=False,
+        arbitrary_types_allowed=True,  # needed in 2.6.0 to allow DigMontage for example
         validate_assignment=True,
         strict=True,  # do not allow float for int for example
         extra="forbid",

mne_bids_pipeline/_config_utils.py

@@ -16,9 +16,9 @@
 from .typing import ArbitraryContrast
 
 try:
-    _keys_arbitrary_contrast = set(ArbitraryContrast.__required_keys__)
+    _set_keys_arbitrary_contrast = set(ArbitraryContrast.__required_keys__)
 except Exception:
-    _keys_arbitrary_contrast = set(ArbitraryContrast.__annotations__.keys())
+    _set_keys_arbitrary_contrast = set(ArbitraryContrast.__annotations__.keys())
 
 
 def get_fs_subjects_dir(config: SimpleNamespace) -> pathlib.Path:
@@ -96,11 +96,14 @@ def get_subjects(config: SimpleNamespace) -> list[str]:
     else:
         s = config.subjects
 
-    subjects = set(s) - set(config.exclude_subjects)
-    # Drop empty-room subject.
-    subjects = subjects - set(["emptyroom"])
+    # Preserve order and remove excluded subjects
+    subjects = [
+        subject
+        for subject in s
+        if subject not in config.exclude_subjects and subject != "emptyroom"
+    ]
 
-    return sorted(subjects)
+    return subjects
 
 
 def get_sessions(config: SimpleNamespace) -> Union[list[None], list[str]]:
@@ -176,7 +179,17 @@ def _get_runs_all_subjects_cached(
 def get_intersect_run(config: SimpleNamespace) -> list[str]:
     """Return the intersection of all the runs of all subjects."""
     subj_runs = get_runs_all_subjects(config)
-    return list(set.intersection(*map(set, subj_runs.values())))
+    # Do not use something like:
+    # list(set.intersection(*map(set, subj_runs.values())))
+    # as it will not preserve order. Instead just be explicit and preserve order.
+    # We could use "sorted", but it's probably better to use the order provided by
+    # the user (if they want to put `runs=["02", "01"]` etc. it's better to use "02")
+    all_runs = list()
+    for runs in subj_runs.values():
+        for run in runs:
+            if run not in all_runs:
+                all_runs.append(run)
+    return all_runs
 
 
 def get_runs(
@@ -429,17 +442,6 @@ def _restrict_analyze_channels(
     return inst
 
 
-def _get_scalp_in_files(cfg: SimpleNamespace) -> dict[str, pathlib.Path]:
-    subject_path = pathlib.Path(cfg.subjects_dir) / cfg.fs_subject
-    seghead = subject_path / "surf" / "lh.seghead"
-    in_files = dict()
-    if seghead.is_file():
-        in_files["seghead"] = seghead
-    else:
-        in_files["t1"] = subject_path / "mri" / "T1.mgz"
-    return in_files
-
-
 def _get_bem_conductivity(cfg: SimpleNamespace) -> tuple[tuple[float], str]:
     if cfg.fs_subject in ("fsaverage", cfg.use_template_mri):
         conductivity = None  # should never be used
@@ -573,7 +575,7 @@ def _validate_contrasts(contrasts: SimpleNamespace) -> None:
             if len(contrast) != 2:
                 raise ValueError("Contrasts' tuples MUST be two conditions")
         elif isinstance(contrast, dict):
-            if not _keys_arbitrary_contrast.issubset(set(contrast.keys())):
+            if not _set_keys_arbitrary_contrast.issubset(set(contrast.keys())):
                 raise ValueError(f"Missing key(s) in contrast {contrast}")
             if len(contrast["conditions"]) != len(contrast["weights"]):
                 raise ValueError(

mne_bids_pipeline/_main.py

@@ -141,7 +141,6 @@ def main():
         steps = (steps,)
 
     on_error = "debug" if debug else None
-    cache = "1" if cache else "0"
 
     processing_stages = []
     processing_steps = []

mne_bids_pipeline/_reject.py

@@ -45,11 +45,11 @@ def _get_reject(
 
     # Only keep thresholds for channel types of interest
     reject = reject.copy()
-    if ch_types == ["eeg"]:
-        ch_types_to_remove = ("mag", "grad")
-    else:
-        ch_types_to_remove = ("eeg",)
-
+    ch_types_to_remove = list()
+    if "meg" not in ch_types:
+        ch_types_to_remove.extend(("mag", "grad"))
+    if "eeg" not in ch_types:
+        ch_types_to_remove.append("eeg")
     for ch_type in ch_types_to_remove:
         try:
             del reject[ch_type]

mne_bids_pipeline/_run.py

@@ -378,11 +378,21 @@ def _prep_out_files(
     *,
     exec_params: SimpleNamespace,
     out_files: dict[str, BIDSPath],
+    check_relative: Optional[pathlib.Path] = None,
 ):
+    if check_relative is None:
+        check_relative = exec_params.deriv_root
     for key, fname in out_files.items():
+        # Sanity check that we only ever write to the derivatives directory
+        fname = pathlib.Path(fname)
+        if not fname.is_relative_to(check_relative):
+            raise RuntimeError(
+                f"Output BIDSPath not relative to expected root {check_relative}:"
+                f"\n{fname}"
+            )
         out_files[key] = _path_to_str_hash(
             key,
-            pathlib.Path(fname),
+            fname,
             method=exec_params.memory_file_method,
             kind="out",
         )
@@ -401,7 +411,7 @@ def _path_to_str_hash(
     assert isinstance(v, pathlib.Path), f'Bad type {type(v)}: {kind}_files["{k}"] = {v}'
     assert v.exists(), f'missing {kind}_files["{k}"] = {v}'
     if method == "mtime":
-        this_hash = v.lstat().st_mtime
+        this_hash = v.stat().st_mtime
     else:
         assert method == "hash"  # guaranteed
         this_hash = hash_file_path(v)

mne_bids_pipeline/steps/freesurfer/_02_coreg_surfaces.py

@@ -10,7 +10,6 @@
 import mne.bem
 
 from ..._config_utils import (
-    _get_scalp_in_files,
     get_fs_subject,
     get_fs_subjects_dir,
     get_subjects,
@@ -22,6 +21,17 @@
 fs_bids_app = Path(__file__).parent / "contrib" / "run.py"
 
 
+def _get_scalp_in_files(cfg: SimpleNamespace) -> dict[str, Path]:
+    subject_path = Path(cfg.fs_subjects_dir) / cfg.fs_subject
+    seghead = subject_path / "surf" / "lh.seghead"
+    in_files = dict()
+    if seghead.is_file():
+        in_files["seghead"] = seghead
+    else:
+        in_files["t1"] = subject_path / "mri" / "T1.mgz"
+    return in_files
+
+
 def get_input_fnames_coreg_surfaces(
     *,
     cfg: SimpleNamespace,
@@ -32,7 +42,7 @@ def get_input_fnames_coreg_surfaces(
 
 def get_output_fnames_coreg_surfaces(*, cfg: SimpleNamespace, subject: str) -> dict:
     out_files = dict()
-    subject_path = Path(cfg.subjects_dir) / cfg.fs_subject
+    subject_path = Path(cfg.fs_subjects_dir) / cfg.fs_subject
     out_files["seghead"] = subject_path / "surf" / "lh.seghead"
     for key in ("dense", "medium", "sparse"):
         out_files[f"head-{key}"] = (
@@ -57,19 +67,21 @@ def make_coreg_surfaces(
     in_files.pop("t1" if "t1" in in_files else "seghead")
     mne.bem.make_scalp_surfaces(
         subject=cfg.fs_subject,
-        subjects_dir=cfg.subjects_dir,
+        subjects_dir=cfg.fs_subjects_dir,
         force=True,
         overwrite=True,
     )
     out_files = get_output_fnames_coreg_surfaces(cfg=cfg, subject=subject)
-    return _prep_out_files(exec_params=exec_params, out_files=out_files)
+    return _prep_out_files(
+        exec_params=exec_params, out_files=out_files, check_relative=cfg.fs_subjects_dir
+    )
 
 
 def get_config(*, config, subject) -> SimpleNamespace:
     cfg = SimpleNamespace(
         subject=subject,
         fs_subject=get_fs_subject(config, subject),
-        subjects_dir=get_fs_subjects_dir(config),
+        fs_subjects_dir=get_fs_subjects_dir(config),
     )
     return cfg
 

mne_bids_pipeline/steps/preprocessing/_05_regress_artifact.py

@@ -76,7 +76,6 @@ def run_regress_artifact(
     out_files["regress"] = bids_path_in.copy().update(
         processing=None,
         split=None,
-        run=None,
         suffix="regress",
         extension=".h5",
     )

mne_bids_pipeline/steps/preprocessing/_09_ptp_reject.py

@@ -187,6 +187,9 @@ def drop_ptp(
         psd = True
     else:
         psd = 30
+    tags = ("epochs", "reject")
+    kind = cfg.reject if isinstance(cfg.reject, str) else "Rejection"
+    title = "Epochs: after cleaning"
     with _open_report(
         cfg=cfg, exec_params=exec_params, subject=subject, session=session
     ) as report:
@@ -201,18 +204,28 @@ def drop_ptp(
                 fig=reject_log.plot(
                     orientation="horizontal", aspect="auto", show=False
                 ),
-                title="Epochs: Autoreject cleaning",
+                title=f"{kind} cleaning",
                 caption=caption,
-                tags=("epochs", "autoreject"),
+                section=title,
+                tags=tags,
                 replace=True,
             )
             del caption
+        else:
+            report.add_html(
+                html=f"<code>{reject}</code>",
+                title=f"{kind} thresholds",
+                section=title,
+                replace=True,
+                tags=tags,
+            )
 
         report.add_epochs(
             epochs=epochs,
-            title="Epochs: after cleaning",
+            title=title,
             psd=psd,
             drop_log_ignore=(),
+            tags=tags,
             replace=True,
         )
     return _prep_out_files(exec_params=exec_params, out_files=out_files)

mne_bids_pipeline/steps/source/_01_make_bem_surfaces.py

@@ -112,7 +112,9 @@ def make_bem_surfaces(
         subject=subject,
         session=session,
     )
-    return _prep_out_files(exec_params=exec_params, out_files=out_files)
+    return _prep_out_files(
+        exec_params=exec_params, out_files=out_files, check_relative=cfg.fs_subjects_dir
+    )
 
 
 def get_config(

mne_bids_pipeline/steps/source/_02_make_bem_solution.py

@@ -69,7 +69,9 @@ def make_bem_solution(
     out_files = get_output_fnames_make_bem_solution(cfg=cfg, subject=subject)
     mne.write_bem_surfaces(out_files["model"], bem_model, overwrite=True)
     mne.write_bem_solution(out_files["sol"], bem_sol, overwrite=True)
-    return _prep_out_files(exec_params=exec_params, out_files=out_files)
+    return _prep_out_files(
+        exec_params=exec_params, out_files=out_files, check_relative=cfg.fs_subjects_dir
+    )
 
 
 def get_config(

mne_bids_pipeline/steps/source/_03_setup_source_space.py

@@ -55,7 +55,9 @@ def run_setup_source_space(
     in_files.clear()  # all used by setup_source_space
     out_files = get_output_fnames_setup_source_space(cfg=cfg, subject=subject)
     mne.write_source_spaces(out_files["src"], src, overwrite=True)
-    return _prep_out_files(exec_params=exec_params, out_files=out_files)
+    return _prep_out_files(
+        exec_params=exec_params, out_files=out_files, check_relative=cfg.fs_subjects_dir
+    )
 
 
 def get_config(