Cross-figure event passing system by wmvanvliet · Pull Request #11685 · mne-tools/mne-python

wmvanvliet · 2023-05-10T11:54:16Z

Proposed in #11678, this PR works on the actual implementation of mne.viz.ui_events. The idea is to run our own event passing system so that figures can talk to each other. For example, if you change the time slider on one figure, the time in other figures is updated accordingly.

TODO:

Follow-up PRs:

@legacy the link_brains function and replace their internals with UI event passing (WIP: Add UI event support for Brain class #11891)
Add example of raw<->epochs linking
Add example of brain linking

Closes #11678

wmvanvliet · 2023-05-10T11:56:24Z

@larsoner is right, we need a better name than just "event" because that is easily confused with MEG event markers.

edit: this change has now been made.

larsoner · 2023-05-10T12:46:47Z

I think I prefer UIEvent. Another option would be FigEvent

mne/viz/ui_events.py

mne/viz/_brain/_brain.py

hoechenberger · 2023-06-08T15:05:34Z

What is the current status / plan here?

wmvanvliet · 2023-06-09T16:41:15Z

Status is that the basic ui-event system seems to be working nicely. However, there are unit tests failing still (that somehow worked when testing locally?).

The plan for this PR is to only implement the ui-event system and the time_change event for Brain and plot_topomap, so it is manageable to review. Then in subsequent PRs we can start implementing ui-events for more of our plots. I've started a TODO section in the PR description up top.

wmvanvliet · 2023-06-09T16:43:49Z

In the upcoming developer sprint, the ui-event system can be a nice thing to work on.

Co-authored-by: Eric Larson <larson.eric.d@gmail.com>

wmvanvliet · 2023-08-14T15:04:26Z

Leaving this for 1.6 sounds good. This PR only makes sense in combination with a bunch of others that integrate the UI events into the interactive figures.

larsoner · 2023-08-14T15:12:05Z

@wmvanvliet FYI I edited the top comment with a tentative plan, feel free to adjust as you see fit

wmvanvliet · 2023-08-16T07:52:29Z

Working on Brain overhaul brought up a new issue with UI events that needs implementing: a way to temporarily disable them.

An issue with the design of the UI event system is how it interacts with GUI elements:

figure receives UI event requesting new value
figure updates plot and GUI elements to match new value
GUI element's set.value() call causes new UI event to fire
infinite loop :(

I thought the loop could be prevented by basically adding boilerplate to the event handlers checking whether new_value == old_value and silently dropping the event if that was the case. However, now I come across GUI elements that subtly change the value. For example:

figure receives UI event requesting a new value of 1.3453
figure updates plot and GUI text box that rounds the value to 1.35
text box value set_value() causes new UI event to fire with value of 1.35
figure receives UI event requesting a new value of 1.35
figure updates plot and GUI text box to 1.35 :(

I guess a more elegant solution is a context manager disable_ui_events that will temporarily silence the production of UI events. An event handler can wrap its GUI updating code with that to prevent loops.

mscheltienne · 2023-08-16T07:57:35Z

A signal blocker, disable_ui_events, similar to QSignalBlocker in Qt seems appropriate.

larsoner · 2023-08-16T13:10:32Z

Good to go again @wmvanvliet ? We cut 1.5 yesterday so this can be merged after one final review I think

larsoner · 2023-08-16T16:37:43Z

mne/viz/tests/test_ui_events.py

+    def callback(event):
+        """Respond to time change event."""
+        global callback_called
+        callback_called = True
+        assert isinstance(event, ui_events.TimeChange)
+        assert event.time == 10.2


FYI to avoid this global business another (quicker/easier usually) approach is to use something mutable like a list:

callback_calls = list() def callback(event): callback_calls.append(event) ... assert len(callback_calls) # or whatever

It automatically deals with this scoping stuff. I'll push a commit to do it then mark for merge when green, but if you really like the global approach I can live with it (and you can put it back in the next PR)!

No strong feelings about it :)

larsoner · 2023-08-16T16:38:26Z

doc/changes/latest.inc

 API changes
 ~~~~~~~~~~~
- None yet
+- Added :mod:`mne.viz.ui_events` API for controlling interactive figures (:gh:`11685` by `Marijn van Vliet`_).


This isn't really an API change the way we use it in latest.inc... it's a feature. API is really for deprecations

* upstream/main: Refactor test_epochs.py::test_split_saving (2 out of 2) (mne-tools#11884) Cross-figure event passing system (mne-tools#11685)

wmvanvliet · 2023-08-17T11:12:19Z

Cool! Now we can build lots of PRs on top of this. Adding interactivity everywhere.

hoechenberger · 2023-08-17T11:13:43Z

Great!

* upstream/main: [pre-commit.ci] pre-commit autoupdate (mne-tools#11911) [BUG, MRG] Remove check on `mne.viz.Brain.add_volume_labels` (mne-tools#11889) Small splits fix (mne-tools#11905) adds niseq package to "Related software" (mne-tools#11909) Minor fixes for ERDS maps example (mne-tools#11904) FIX: Fix pyvista rendering (mne-tools#11896) BUG: Fix epoch splits naming (mne-tools#11876) ENH: Use section-title for HTML anchors in Report (mne-tools#11890) CI: Deploy [circle deploy] MAINT: Clean up whats_new and doc versions (mne-tools#11888) Refactor test_epochs.py::test_split_saving (2 out of 2) (mne-tools#11884) Cross-figure event passing system (mne-tools#11685) MAINT: Post-release deprecations, updates [circle deploy] (mne-tools#11887) MAINT: Release 1.5.0 (mne-tools#11886) [pre-commit.ci] pre-commit autoupdate (mne-tools#11883) Refactor test_epochs.py::test_split_saving (1 out of 2) (mne-tools#11880) FIX: Missing Saccade information in Eyelink File (mne-tools#11877) Improve drawing of annotations with matplotlib (mne-tools#11855) MAINT: Work around NumPy deprecation (mne-tools#11878)

wmvanvliet added 3 commits May 10, 2023 09:36

First version of the events API

28722d2

Add some interaction capabilities to Brain and topomap

47076b5

Add basic example of linking the event channels of two figures

e2ff979

wmvanvliet added 2 commits May 10, 2023 15:40

Event -> UIEvent

aed7fad

Update UI-events example

76da046

wmvanvliet added API GUI labels May 10, 2023

larsoner reviewed May 10, 2023

View reviewed changes

mne/viz/ui_events.py Show resolved Hide resolved

wmvanvliet added 13 commits May 10, 2023 16:58

Better cleanup?

c739527

Attempt at making the example work better

1c3dbb8

Some tweaks to docs and mark the global UI event channels private

601b5c3

Add unit tests for core UI event system

5a3b6fa

black

6026fe9

More black

216831c

Even more black

20758df

docstyle

c47da0a

Fix nesting

08c2c2d

Remove some remaining instances of update_width=True

a919d72

Small tweak to unit test

a516c00

Make events work with the notebook Brain renderer too.

f5f6e8c

Allow selecting which events to link across figures.

6108aaf

hoechenberger reviewed May 29, 2023

View reviewed changes

mne/viz/_brain/_brain.py Show resolved Hide resolved

wmvanvliet added 3 commits June 6, 2023 09:45

Fix some more callbacks

8d706e7

Change function argument name

06008a2

Remove debug print statement

3371922

Apply suggestions from code review

d3a880f

Co-authored-by: Eric Larson <larson.eric.d@gmail.com>

larsoner added this to the 1.6 milestone Aug 14, 2023

Move publication_figure and ui_events examples to the tutorial section

7bc5890

wmvanvliet self-assigned this Aug 15, 2023

Merge branch 'main' into events

e87c82e

wmvanvliet changed the title ~~[MRG] Cross-figure event passing system~~ Cross-figure event passing system Aug 16, 2023

wmvanvliet marked this pull request as draft August 16, 2023 07:40

wmvanvliet added 4 commits August 16, 2023 11:09

Add disable_ui_events context manager

bf53637

Remove debug print() calls

bb724f3

Merge branch 'main' into events

ecc4b3d

Update what's new

02f4bc4

wmvanvliet marked this pull request as ready for review August 16, 2023 09:20

MAINT: Avoid globals

aead573

larsoner reviewed Aug 16, 2023

View reviewed changes

FIX: Where

6bd2195

larsoner enabled auto-merge (squash) August 16, 2023 16:39

larsoner merged commit 0e68aba into mne-tools:main Aug 16, 2023

wmvanvliet deleted the events branch August 17, 2023 11:13

hoechenberger mentioned this pull request Oct 3, 2023

MAINT: Use github_changelog_generator for release notes #12051

Closed

7 tasks

snwnde pushed a commit to snwnde/mne-python that referenced this pull request Mar 20, 2024

Cross-figure event passing system (mne-tools#11685)

596a46c

Uh oh!

Conversation

wmvanvliet commented May 10, 2023 • edited by mmagnuski Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wmvanvliet commented May 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

larsoner commented May 10, 2023

Uh oh!

Uh oh!

Uh oh!

hoechenberger commented Jun 8, 2023

Uh oh!

wmvanvliet commented Jun 9, 2023

Uh oh!

wmvanvliet commented Jun 9, 2023

Uh oh!

wmvanvliet commented Aug 14, 2023

Uh oh!

larsoner commented Aug 14, 2023

Uh oh!

wmvanvliet commented Aug 16, 2023

Uh oh!

mscheltienne commented Aug 16, 2023

Uh oh!

larsoner commented Aug 16, 2023

Uh oh!

larsoner Aug 16, 2023

Choose a reason for hiding this comment

Uh oh!

wmvanvliet Aug 17, 2023

Choose a reason for hiding this comment

Uh oh!

larsoner Aug 16, 2023

Choose a reason for hiding this comment

Uh oh!

wmvanvliet commented Aug 17, 2023

Uh oh!

hoechenberger commented Aug 17, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

wmvanvliet commented May 10, 2023 •

edited by mmagnuski

Loading

wmvanvliet commented May 10, 2023 •

edited

Loading