Skip to content

ENH: Route evoked.plot() through MNELineFigure#13795

Open
PragnyaKhandelwal wants to merge 6 commits intomne-tools:mainfrom
PragnyaKhandelwal:fix-evoked-mpl-figure-13747
Open

ENH: Route evoked.plot() through MNELineFigure#13795
PragnyaKhandelwal wants to merge 6 commits intomne-tools:mainfrom
PragnyaKhandelwal:fix-evoked-mpl-figure-13747

Conversation

@PragnyaKhandelwal
Copy link
Copy Markdown
Contributor

Reference issue (if any)
Closes #13747
Related to #7751

What does this implement/fix?
When evoked.plot() creates its own figure (axes=None), it now routes through _line_figure() to instantiate MNELineFigure instead of a plain Matplotlib figure.

This aligns the default 2D plotting path with the refactor direction discussed in #7751, while keeping behavior unchanged when users pass custom axes.

Scope is intentionally limited:

evoked.plot_white() is out of scope
evoked.plot_joint() is out of scope
A regression test was added to assert MNELineFigure instantiation for the default evoked.plot() path.

Additional information
This is a localized, backward-compatible enhancement intended as an incremental step toward #7751, not a full closure of that broader refactor.

When evoked.plot() creates its own figure (axes=None), now routes through
_line_figure() to instantiate MNELineFigure instead of a plain matplotlib
figure. This aligns with the 2D plotting figure-class refactor direction
discussed in mne-tools#7751.

Behavior unchanged for custom axes. Both plot_white() and plot_joint()
remain out of scope here.

Adds regression test to assert MNELineFigure instantiation for default path.
Closes mne-tools#13747
@PragnyaKhandelwal PragnyaKhandelwal marked this pull request as ready for review March 29, 2026 10:19
@PragnyaKhandelwal
Copy link
Copy Markdown
Contributor Author

Hi @larsoner, @drammock — just wanted to know the preferred direction for this refactor.
I’ve kept the scope intentionally limited to the default evoked.plot() path to keep this PR manageable. However, I’m happy to expand this to include plot_white() and plot_joint() if you’d prefer to see the full MNELineFigure transition for Evoked handled in a single PR.
Would you prefer I finish the rest of the Evoked methods here, or should we keep this as an incremental step and handle the others in follow-up PRs?

@PragnyaKhandelwal
Copy link
Copy Markdown
Contributor Author

Addressed review points @larsoner... could you please take another look when convenient? Thanks.

@PragnyaKhandelwal
Copy link
Copy Markdown
Contributor Author

Hi @larsoner, @drammock.......just a friendly ping. All checks are green and I’ve addressed the feedback from the previous review.
I’m looking forward to getting this merged so I can carry the MNELineFigure refactor over to evoked.plot_white() and evoked.plot_joint() in follow-up PRs. Let me know if there's anything else you'd like me to tweak!

@drammock
Copy link
Copy Markdown
Member

drammock commented Apr 7, 2026

Hi @PragnyaKhandelwal, as this changes plotting behavior, it needs visual inspection of the plots (not just the code) to make sure things still look correct. You can help speed up the review by:

  1. posting before/after screenshots of how evoked plots look
  2. finding a tutorial in our docs that uses the new code, and make a tiny change to that tutorial (even just changing a line break is enough)... this will trigger our CI to re-execute that tutorial so we can see the plot in-situ
  3. after step 2, find the CI called "check the rendered docs here" and find the URL for the rendered tutorial, and paste that link in a comment here so it's quick for us to find it.
  4. provide a short code block that we can copy-paste to get the relevant figure locally, so we can interact with it (not just view the static version on the CI)

(step 1 could be skipped in this case actually. Sometimes a screenshot is enough, but in this case we need to verify interactivity is preserved)

@PragnyaKhandelwal
Copy link
Copy Markdown
Contributor Author

Thanks for the guidance @drammock! I've completed the visual and architectural verification. Everything aligns with the current main branch while successfully migrating to the new figure class.

  1. Architectural Verification
    I've verified locally that the evoked.plot() default path now correctly instantiates MNELineFigure. You can use this snippet to verify the class and interactivity:
import mne
from mne.datasets import sample
from mne.viz._mpl_figure import MNELineFigure

data_path = sample.data_path()
evoked_fname = data_path / 'MEG' / 'sample' / 'sample_audvis-ave.fif'
evoked = mne.read_evokeds(evoked_fname, condition='Left Auditory')

fig = evoked.plot(spatial_colors=True)

print(f"Figure Class: {type(fig)}")
assert isinstance(fig, MNELineFigure)
print("Figure is an MNELineFigure `instance.")
  1. Visual Consistency & Interactivity
    The rendering is identical to the legacy version. I've also verified that channel-picking remains fully functional, which confirms the event handling is correctly set up in the new class.

Before (main)
image

After (this pr)
image

  1. Rendered Documentation
    I've triggered a rebuild of the tutorials. You can inspect the plots in-situ here:
    https://output.circle-artifacts.com/output/job/696480af-a619-47a9-9180-11a9583c787d/artifacts/0/html/auto_tutorials/evoked/10_evoked_overview.html#sphx-glr-auto-tutorials-evoked-10-evoked-overview-py

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: evoked.plot() still bypasses MNELineFigure path (related to #7751)

3 participants