Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MAINT: Refactor _get_raw_paths #749

Merged
merged 28 commits into from
Jul 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/source/settings/general.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
- eeg_template_montage
- drop_channels
- reader_extra_params
- read_raw_bids_verbose
- analyze_channels
- plot_psd_for_runs
- n_jobs
Expand Down
7 changes: 4 additions & 3 deletions docs/source/v1.4.md.inc
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,20 @@
### :new: New features & enhancements

- Add movement compensation and cHPI filtering to the Maxwell filtering step, along with additional configuration options (#747 by @larsoner)
- Add option to specify `ssp_ecg_channel` to override the default value (#747 by @larsoner)
- Add option to specify [`ssp_ecg_channel`]([mne_bids_pipeline._config.ssp_ecg_channel) to override the default value (#747 by @larsoner)
- Add option [`read_raw_bids_verbose`]([mne_bids_pipeline._config.read_raw_bids_verbose) to set the verbosity level when using `read_raw_bids` to suppress known warnings (#749 by @larsoner)

[//]: # (### :warning: Behavior changes)

[//]: # (- Whatever (#000 by @whoever))

### :medical_symbol: Code health

- Refactor code to deduplicate keyword-passing (#746 by @larsoner)
- Refactor code to deduplicate keyword-passing and improve internal logic (#746, #749 by @larsoner)

### :bug: Bug fixes

- Fix bug when `mf_reference_run != runs[0]` (#742 by @larsoner)
- Fix bug when [`mf_reference_run != runs[0]`]([mne_bids_pipeline._config.mf_reference_run) (#742 by @larsoner)
- Fix bug with too many JSON files found during empty room matching (#743 by @allermat)
- Fix bug with outdated info on ch_types config option (#745 by @allermat)
- Fix bug where SSP projectors were not added to the report (#747 by @larsoner)
Expand Down
8 changes: 8 additions & 0 deletions mne_bids_pipeline/_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,14 @@
```
"""

read_raw_bids_verbose: Optional[Literal["error"]] = None
Copy link
Member Author

@larsoner larsoner Jun 30, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added this one because I was getting a bunch of warnings when processing ds000117:

/home/larsoner/python/mne-bids-pipeline/mne_bids_pipeline/_import_data.py:226: RuntimeWarning: The events file, /mnt/bakraid/data/mne_data/ds000117-full/sub-01/ses-meg/meg/sub-01_ses-meg_task-facerecognition_run-04_events.tsv, contains a "stim_type" column. This column should be renamed to "trial_type" for BIDS compatibility.
  raw = read_raw_bids(
/home/larsoner/python/mne-bids-pipeline/mne_bids_pipeline/_import_data.py:226: RuntimeWarning: Did not find any channels.tsv associated with sub-01_ses-meg_task-facerecognition_run-04.

The search_str was "/mnt/bakraid/data/mne_data/ds000117-full/sub-01/**/meg/sub-01_ses-meg*channels.tsv"
  raw = read_raw_bids(
/home/larsoner/python/mne-bids-pipeline/mne_bids_pipeline/_import_data.py:226: RuntimeWarning: Did not find any meg.json associated with sub-01_ses-meg_task-facerecognition_run-04.

The search_str was "/mnt/bakraid/data/mne_data/ds000117-full/sub-01/**/meg/sub-01_ses-meg*meg.json"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you mention this new setting in the changelog, please?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

"""
Verbosity level to pass to `read_raw_bids(..., verbose=read_raw_bids_verbose)`.
If you know your dataset will contain files that are not perfectly BIDS
compliant (e.g., "Did not find any meg.json..."), you can set this to
`'error'` to suppress warnings emitted by read_raw_bids.
"""

###############################################################################
# BREAK DETECTION
# ---------------
Expand Down
75 changes: 42 additions & 33 deletions mne_bids_pipeline/_config_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,16 +118,6 @@ def get_sessions(config: SimpleNamespace) -> Union[List[None], List[str]]:
return sessions


@functools.lru_cache(maxsize=None)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic here was backward I think. The "public" get_runs_all_subjects should just cache under the hood so anytime we use it we benefit.

def _get_runs_all_subjects_cached(
**config_dict: Dict[str, Any],
) -> Dict[str, Union[List[None], List[str]]]:
config = SimpleNamespace(**config_dict)
# Sometimes we check list equivalence for ch_types, so convert it back
config.ch_types = list(config.ch_types)
return get_runs_all_subjects(config)


def get_runs_all_subjects(
config: SimpleNamespace,
) -> Dict[str, Union[List[None], List[str]]]:
Expand All @@ -139,7 +129,26 @@ def get_runs_all_subjects(
for each subject asked in the configuration file
(and not for each subject present in the bids_path).
"""
# We cannot use get_subjects() because if there is just one subject
# Use caching under the hood for speed
return copy.deepcopy(
_get_runs_all_subjects_cached(
bids_root=config.bids_root,
data_type=config.data_type,
ch_types=tuple(config.ch_types),
subjects=tuple(config.subjects) if config.subjects != "all" else "all",
exclude_subjects=tuple(config.exclude_subjects),
exclude_runs=tuple(config.exclude_runs) if config.exclude_runs else None,
)
)


@functools.lru_cache(maxsize=None)
def _get_runs_all_subjects_cached(
**config_dict: Dict[str, Any],
) -> Dict[str, Union[List[None], List[str]]]:
config = SimpleNamespace(**config_dict)
# Sometimes we check list equivalence for ch_types, so convert it back
config.ch_types = list(config.ch_types)
subj_runs = dict()
for subject in get_subjects(config):
# Only traverse through the current subject's directory
Expand Down Expand Up @@ -193,15 +202,7 @@ def get_runs(
return [None]

runs = copy.deepcopy(config.runs)

subj_runs = _get_runs_all_subjects_cached(
bids_root=config.bids_root,
data_type=config.data_type,
ch_types=tuple(config.ch_types),
subjects=tuple(config.subjects) if config.subjects != "all" else "all",
exclude_subjects=tuple(config.exclude_subjects),
exclude_runs=tuple(config.exclude_runs) if config.exclude_runs else None,
)
subj_runs = get_runs_all_subjects(config=config)
valid_runs = subj_runs[subject]

if len(get_subjects(config)) > 1:
Expand Down Expand Up @@ -239,24 +240,28 @@ def get_runs_tasks(
config: SimpleNamespace,
subject: str,
session: Optional[str],
include_noise: bool = True,
) -> List[Tuple[str]]:
"""Get (run, task) tuples for all runs plus (maybe) rest."""
from ._import_data import _get_bids_path_in
from ._import_data import _get_noise_path, _get_rest_path

runs = get_runs(config=config, subject=subject)
tasks = [config.task] * len(runs)
if config.process_rest and not config.task_is_rest:
run, task = None, "rest"
bids_path_in = _get_bids_path_in(
cfg=config,
subject=subject,
session=session,
run=run,
task=task,
)
if bids_path_in.fpath.exists():
tasks = [get_task(config=config)] * len(runs)
kwargs = dict(
cfg=config,
subject=subject,
session=session,
kind="orig",
add_bads=False,
)
if _get_rest_path(**kwargs):
runs.append(None)
tasks.append("rest")
if include_noise:
mf_reference_run = get_mf_reference_run(config=config)
if _get_noise_path(mf_reference_run=mf_reference_run, **kwargs):
runs.append(None)
tasks.append("rest")
tasks.append("noise")
return tuple(zip(runs, tasks))


Expand Down Expand Up @@ -600,3 +605,7 @@ def _bids_kwargs(*, config: SimpleNamespace) -> dict:
bids_root=config.bids_root,
deriv_root=config.deriv_root,
)


def _do_mf_autobad(*, cfg: SimpleNamespace) -> bool:
return cfg.find_noisy_channels_meg or cfg.find_flat_channels_meg
Loading