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

Fix handling of analyze_channels for EEG data, where some steps (like ERP calculation) would previously fail #883

Merged
merged 4 commits into from Mar 12, 2024
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion docs/source/v1.7.md.inc
Expand Up @@ -16,7 +16,9 @@

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

[//]: # (### :bug: Bug fixes)
### :bug: Bug fixes

- Fixed an error when using [`analyze_channels`][mne_bids_pipeline._config.analyze_channels] with EEG data, where e.g. ERP creation didn't work. (#883 by @hoechenberger)

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

Expand Down
6 changes: 4 additions & 2 deletions mne_bids_pipeline/_config.py
Expand Up @@ -3,7 +3,7 @@
from collections.abc import Sequence
from typing import Annotated, Any, Callable, Literal, Optional, Union

from annotated_types import Ge, Interval, Len
from annotated_types import Ge, Interval, Len, MinLen
from mne import Covariance
from mne_bids import BIDSPath

Expand Down Expand Up @@ -382,7 +382,9 @@
```
"""

analyze_channels: Union[Literal["all", "ch_types"], Sequence["str"]] = "ch_types"
analyze_channels: Union[
Literal["all", "ch_types"], Annotated[Sequence["str"], MinLen(1)]
] = "ch_types"
"""
The names of the channels to analyze during ERP/ERF and time-frequency analysis
steps. For certain paradigms, e.g. EEG ERP research, it is common to constrain
Expand Down
31 changes: 15 additions & 16 deletions mne_bids_pipeline/_config_utils.py
Expand Up @@ -423,22 +423,21 @@ def get_mf_ctc_fname(
def _restrict_analyze_channels(
inst: RawEpochsEvokedT, cfg: SimpleNamespace
) -> RawEpochsEvokedT:
if cfg.analyze_channels:
larsoner marked this conversation as resolved.
Show resolved Hide resolved
analyze_channels = cfg.analyze_channels
if cfg.analyze_channels == "ch_types":
analyze_channels = cfg.ch_types
inst.apply_proj()
# We special-case the average reference here to work around a situation
# where e.g. `analyze_channels` might contain only a single channel:
# `concatenate_epochs` below will then fail when trying to create /
# apply the projection. We can avoid this by removing an existing
# average reference projection here, and applying the average reference
# directly – without going through a projector.
elif "eeg" in cfg.ch_types and cfg.eeg_reference == "average":
inst.set_eeg_reference("average")
else:
inst.apply_proj()
inst.pick(analyze_channels)
analyze_channels = cfg.analyze_channels
if cfg.analyze_channels == "ch_types":
analyze_channels = cfg.ch_types
inst.apply_proj()
# We special-case the average reference here to work around a situation
# where e.g. `analyze_channels` might contain only a single channel:
# `concatenate_epochs` below will then fail when trying to create /
# apply the projection. We can avoid this by removing an existing
# average reference projection here, and applying the average reference
# directly – without going through a projector.
elif "eeg" in cfg.ch_types and cfg.eeg_reference == "average":
inst.set_eeg_reference("average")
else:
inst.apply_proj()
inst.pick(analyze_channels)
return inst


Expand Down
2 changes: 2 additions & 0 deletions mne_bids_pipeline/steps/sensor/_01_make_evoked.py
Expand Up @@ -11,6 +11,7 @@
_pl,
_restrict_analyze_channels,
get_all_contrasts,
get_eeg_reference,
get_sessions,
get_subjects,
)
Expand Down Expand Up @@ -172,6 +173,7 @@ def get_config(
contrasts=get_all_contrasts(config),
noise_cov=_sanitize_callable(config.noise_cov),
analyze_channels=config.analyze_channels,
eeg_reference=get_eeg_reference(config),
ch_types=config.ch_types,
report_evoked_n_time_points=config.report_evoked_n_time_points,
**_bids_kwargs(config=config),
Expand Down
2 changes: 2 additions & 0 deletions mne_bids_pipeline/steps/sensor/_06_make_cov.py
Expand Up @@ -13,6 +13,7 @@
from ..._config_utils import (
_bids_kwargs,
_restrict_analyze_channels,
get_eeg_reference,
get_noise_cov_bids_path,
get_sessions,
get_subjects,
Expand Down Expand Up @@ -292,6 +293,7 @@ def get_config(
conditions=config.conditions,
contrasts=config.contrasts,
analyze_channels=config.analyze_channels,
eeg_reference=get_eeg_reference(config),
noise_cov_method=config.noise_cov_method,
**_bids_kwargs(config=config),
)
Expand Down
70 changes: 70 additions & 0 deletions mne_bids_pipeline/tests/configs/config_ERP_CORE.py
Expand Up @@ -212,6 +212,41 @@
}

eeg_reference = ["P9", "P10"]
# Analyze all EEG channels -- we only specify the channels here for the purpose of
# demonstration
analyze_channels = [
Comment on lines 214 to +217
Copy link
Member Author

Choose a reason for hiding this comment

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

test with custom reference

"FP1",
"F3",
"F7",
"FC3",
"C3",
"C5",
"P3",
"P7",
"P9",
"PO7",
"PO3",
"O1",
"Oz",
"Pz",
"CPz",
"FP2",
"Fz",
"F4",
"F8",
"FC4",
"FCz",
"Cz",
"C4",
"C6",
"P4",
"P8",
"P10",
"PO8",
"PO4",
"O2",
]

epochs_tmin = -0.2
epochs_tmax = 0.8
baseline = (None, 0)
Expand All @@ -224,6 +259,41 @@
}

eeg_reference = "average"
# Analyze all EEG channels -- we only specify the channels here for the purpose of
# demonstration
analyze_channels = [
Comment on lines 261 to +264
Copy link
Member Author

Choose a reason for hiding this comment

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

test with average reference

"FP1",
"F3",
"F7",
"FC3",
"C3",
"C5",
"P3",
"P7",
"P9",
"PO7",
"PO3",
"O1",
"Oz",
"Pz",
"CPz",
"FP2",
"Fz",
"F4",
"F8",
"FC4",
"FCz",
"Cz",
"C4",
"C6",
"P4",
"P8",
"P10",
"PO8",
"PO4",
"O2",
]

ica_n_components = 30 - 1
for i in range(1, 180 + 1):
orig_name = f"stimulus/{i}"
Expand Down