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.

Sign up for GitHub

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

Jump to bottom

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

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1d681e6
Fix hadling of `analyze_channels` for EEG data, where some steps (lik…
hoechenberger Mar 12, 2024
54e5269
Add example test case
hoechenberger Mar 12, 2024
4e44840
Remove unnecessary constraint (always true)
hoechenberger Mar 12, 2024
41b3358
FIX: MinLen
larsoner Mar 12, 2024
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
4 changes: 3 additions & 1 deletion docs/source/v1.7.md.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
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
View file
Open in desktop
Original file line number Diff line number Diff line change
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
View file
Open in desktop
Original file line number Diff line number Diff line change
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
View file
Open in desktop
Original file line number Diff line number Diff line change
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
View file
Open in desktop
Original file line number Diff line number Diff line change
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
View file
Open in desktop
Original file line number Diff line number Diff line change
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
Loading