Skip to content

actually use GFP for EEG channels in plot_compare_evokeds#12410

Merged
drammock merged 7 commits intomne-tools:mainfrom
drammock:gfp-rms
Feb 2, 2024
Merged

actually use GFP for EEG channels in plot_compare_evokeds#12410
drammock merged 7 commits intomne-tools:mainfrom
drammock:gfp-rms

Conversation

@drammock
Copy link
Copy Markdown
Member

@drammock drammock commented Feb 1, 2024

fixes #9022

Copy link
Copy Markdown
Member Author

@drammock drammock left a comment

Choose a reason for hiding this comment

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

left some comments to hopefully speed up review

Comment on lines -63 to -66
def _identity_function(x):
return x


Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

this was moved because it is general-purpose and now used outside of this file

Comment on lines +405 to +424
@pytest.mark.parametrize(
"combine,vlines,title,picks",
(
pytest.param(None, [0.1, 0.2], "MEG 0113", "MEG 0113", id="singlepick"),
pytest.param("mean", [], "(mean)", "mag", id="mag-mean"),
pytest.param("gfp", "auto", "(GFP)", "eeg", id="eeg-gfp"),
pytest.param(None, "auto", "(RMS)", ["MEG 0113", "MEG 0112"], id="meg-rms"),
pytest.param(
"std", "auto", "(std. dev.)", ["MEG 0113", "MEG 0112"], id="meg-std"
),
pytest.param(
lambda x: np.min(x, axis=1), "auto", "MEG 0112", [0, 1], id="intpicks"
),
),
)
def test_plot_compare_evokeds_title(evoked, picks, vlines, combine, title):
"""Test title generation by plot_compare_evokeds()."""
# test picks, combine, and vlines (1-channel pick also shows sensor inset)
fig = plot_compare_evokeds(evoked, picks=picks, vlines=vlines, combine=combine)
assert fig[0].axes[0].get_title().endswith(title)
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

this test is just a portion extracted from the following test, to make the params easier to follow. One new param was added (eeg-gfp) and one expected title value changes (meg-rms will now say "(RMS)" instead of "(GFP)")

Comment on lines +2370 to +2372
# marginal median that is safe for complex values:
if "median" in valid:
combine_dict["median"] = partial(_median_complex, axis=axis)
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

the switch to marginal median isn't strictly necessary for this PR but will be for #11282; easier to just add it now.

Comment on lines +2336 to +2339
axis=1,
valid=("mean", "median", "std", "gfp"),
ch_type=None,
keepdims=False,
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

addition of axis, valid, and keepdims are to make this function useful for TFRs (#11282)

@drammock drammock changed the title use RMS for MEG chans when GFP requested in plot_compare_evokeds actually use GFP for EEG channels in plot_compare_evokeds Feb 1, 2024
Copy link
Copy Markdown
Member

@larsoner larsoner left a comment

Choose a reason for hiding this comment

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

Approved pending addressing of @hoechenberger's comments, feel free to mark for merge-when-green when you do @drammock

@drammock drammock enabled auto-merge (squash) February 2, 2024 15:07
@drammock drammock merged commit d8ea2f5 into mne-tools:main Feb 2, 2024
@drammock drammock deleted the gfp-rms branch February 2, 2024 15:56
snwnde pushed a commit to snwnde/mne-python that referenced this pull request Mar 20, 2024
…12410)

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

viz.plot_compare_evokeds() calculates RMS for all channel types, but claims to be calculating GFP

3 participants