Skip to content

ENH: Show the current time as a vertical line in plot_evoked_topo#14032

Merged
larsoner merged 5 commits into
mne-tools:mainfrom
wmvanvliet:plot_evoked_topo-timechange
Jul 8, 2026
Merged

ENH: Show the current time as a vertical line in plot_evoked_topo#14032
larsoner merged 5 commits into
mne-tools:mainfrom
wmvanvliet:plot_evoked_topo-timechange

Conversation

@wmvanvliet

Copy link
Copy Markdown
Contributor

A follow-up to #13968. This adds the time cursor also to the parent figure of mne.viz.plot_evoked_topo. This behavior mimicks neuromag's xplotter program and was a requested feature in our research group.

image

@wmvanvliet wmvanvliet requested a review from larsoner as a code owner July 7, 2026 13:45
@wmvanvliet

Copy link
Copy Markdown
Contributor Author

@Gnefil Could you perform a review on this?

Comment thread mne/viz/topo.py
_on_time_change,
fig=fig,
tmin=np.min([t[0] for t in times]),
tmax=np.max([t[-1] for t in times]),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So we choose to take the highest upperbound and the lowest lowerbound, and hide vertical lines for these capped by their times range.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yeah

@Gnefil Gnefil left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good to me, clean implementation, and not overloaded test.

@larsoner

larsoner commented Jul 8, 2026

Copy link
Copy Markdown
Member

My only question was going to be about coverage since I don't see any new test, but we must already be covering this code path because codecov says 90.47% of diff hit. Also LGTM so in it goes, thanks @wmvanvliet for the improvement and @Gnefil for the quick review!

@larsoner larsoner merged commit ddcefca into mne-tools:main Jul 8, 2026
32 checks passed
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.

3 participants