Skip to content

Conversation

@mistraube
Copy link
Contributor

@mistraube mistraube commented Nov 8, 2025

Reference issue (if any)

Fixes #13271.

What does this implement/fix?

Fix a regression with mne.viz.plot_evoked() when using gfp="only" or gfp=True.

Plots based on: https://mne.tools/stable/auto_tutorials/evoked/30_eeg_erp.html#sphx-glr-auto-tutorials-evoked-30-eeg-erp-py

l_aud.plot(gfp="only")

Before:
gfp_before
After:
gfp_after_v2

l_aud.plot(gfp=True, spatial_colors=True, ylim=dict(eeg=[-12, 12]))

Before:
evoked_before
After:
evoked_after_v2

Additional information

I tried to identify any other plotting code that might be negatively affected by this change, but I didn't find any.

@larsoner
Copy link
Member

Hmmm in your first plot there is a gap at the bottom, ideally the ylim would start at zero since GFP must be non-negative...

I think in the non-GFP-only case we must do something to set the ylim properly. What is it? Could we extend that logic instead, setting the ylim manually in the GFP-only case? That seems like maybe better/more consistent behavior. Maybe something like ylim = (0, (1.1 * np.max(gfp)) or 1) or similar would work...

@mistraube
Copy link
Contributor Author

Ok, then actually the behavior before this issue was introduced was already not correct. It also has the gap in the gfp-only case. See: https://mne.tools/1.5/auto_tutorials/evoked/30_eeg_erp.html#sphx-glr-auto-tutorials-evoked-30-eeg-erp-py

I'll look into it again.

@mistraube
Copy link
Contributor Author

mistraube commented Nov 16, 2025

I added a commit to address the issue with the gap at the bottom and updated the plots in the initial post.

Also, I noticed another issue that was already present before the changes in this PR. The curve is plotted above the y-axis spine. But that is not limited to the GFP-only case, the channels are also plotted above the y-axis in other evoked plots.

Bildschirmfoto_20251116_085855

Maybe this should be addressed in another PR to keep this one clean. If I find some time I can look into it, but currently I am very busy. I guess we just need to adjust the zorder of the axis spines.

@drammock
Copy link
Member

I've pushed one more commit to make https://mne.tools/dev/auto_tutorials/evoked/30_eeg_erp.html#global-field-power-gfp re-render, just so we can confirm that the plot looks correct again when that tutorial is built by the CIs. Assuming it looks good, +1 for merge

@drammock
Copy link
Member

(I did it myself @mistraube because for security reasons, in our repository CircleCI doesn't run on PRs from people who don't have CircleCI accounts)

@larsoner larsoner enabled auto-merge (squash) November 19, 2025 23:39
@larsoner larsoner merged commit 587c99f into mne-tools:main Nov 19, 2025
32 checks passed
@larsoner
Copy link
Member

I had a look and it seemed correct, thanks @mistraube !

@mistraube mistraube deleted the viz branch November 20, 2025 11:36
larsoner added a commit to zhijingz/mne-python that referenced this pull request Nov 20, 2025
* upstream/main:
  ENH: Parse EyeLink BUTTON Events (e.g. from a game controller) (mne-tools#13499)
  FIX: Fix regression with mne.viz.plot_evoked (mne-tools#13481)
  DOC: related software doc (mne-tools#13498)
  FIX: Replace use of deprecated Numpy func in GDF reader  (Supersedes mne-tools#13415) (mne-tools#13497)
  DOC: Link to membership from governance page (mne-tools#13496)
  [pre-commit.ci] pre-commit autoupdate (mne-tools#13495)
  BUG: Fix issue with Montage.plot (mne-tools#13494)
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.

ylims broken for evoked.plot(gfp='only')

3 participants