[ENH] Draft GUI for labeling components #66

adam2392 · 2022-06-15T00:32:56Z

PR Description

Addresses: #12

From MNE-Python, I understand there are two parts of a GUI API pattern that we can follow:

actual GUI Python class: https://github.com/mne-tools/mne-python/blob/04b5ed9afb7722e72e43dd2082bdfc5e4c1c9d52/mne/gui/_ieeg_locate_gui.py#L106-L122
functional API: https://github.com/mne-tools/mne-python/blob/04b5ed9afb7722e72e43dd2082bdfc5e4c1c9d52/mne/gui/__init__.py#L191-L229

I'm not very familiar with a good workflow for getting this to work properly so can definitely use help :p

Details of the GUI

The GUI will I think house 3 plots, stemming from ICA.plot_properties() (https://mne.tools/stable/generated/mne.preprocessing.ICA.html#mne.preprocessing.ICA.plot_properties). There should be basic saving/loading functions of course that interface with the BIDS files on disc.

Merge checklist

Maintainer, please confirm the following before merging:

codecov · 2022-06-15T00:37:45Z

Codecov Report

Merging #66 (b3e2f1f) into main (c767cac) will decrease coverage by 5.94%.
The diff coverage is 72.56%.

@@            Coverage Diff             @@
##             main      #66      +/-   ##
==========================================
- Coverage   96.58%   90.63%   -5.95%     
==========================================
  Files          19       23       +4     
  Lines         673      897     +224     
==========================================
+ Hits          650      813     +163     
- Misses         23       84      +61

Impacted Files	Coverage Δ
mne_icalabel/commands/mne_gui_ic_annotation.py	`0.00% <0.00%> (ø)`
mne_icalabel/conftest.py	`100.00% <ø> (+2.94%)`	⬆️
mne_icalabel/utils/_checks.py	`46.42% <21.05%> (-53.58%)`	⬇️
mne_icalabel/gui/_label_components.py	`86.11% <86.11%> (ø)`
mne_icalabel/gui/__init__.py	`88.88% <88.88%> (ø)`
mne_icalabel/iclabel/label_components.py	`95.65% <92.85%> (-4.35%)`	⬇️
mne_icalabel/__init__.py	`100.00% <100.00%> (ø)`
mne_icalabel/annotation/bids.py	`85.24% <100.00%> (+3.24%)`	⬆️
mne_icalabel/config.py	`100.00% <100.00%> (ø)`
mne_icalabel/utils/__init__.py	`100.00% <100.00%> (ø)`
... and 1 more

Help us with your feedback. Take ten seconds to tell us how you rate us.

mscheltienne

That was not what I had in mind ahah I was thinking of a GUI that loads components one by one, not necessarily from the same (raw, ica). But that fits way better in the API of this package, so I like it!
I'll add a second file with a second iteration to this PR, I got some time today :)

mne_icalabel/gui/__init__.py

mne_icalabel/gui/_label_components.py

mscheltienne · 2022-06-15T08:42:32Z

mne_icalabel/gui/_label_components.py

+def _make_ts_plot(width=4, height=4, dpi=300):
+    """Make subplot for the component time-series."""
+    fig = Figure(figsize=(width, height), dpi=dpi)
+    canvas = FigureCanvas(fig)
+    ax = fig.subplots()
+    fig.subplots_adjust(bottom=0, left=0, right=1, top=1, wspace=0, hspace=0)
+    ax.set_facecolor("k")
+    # clean up excess plot text, invert
+    ax.invert_yaxis()
+    ax.set_xticks([])
+    ax.set_yticks([])
+    return canvas, fig


For the time-series plot, we need to be able to scroll in time, and probably also zoom on both the X and Y-axis.
How about maybe looking into the mne-qt-browser, maybe we can just load the widgets it has?

mne_icalabel/gui/_label_components.py

adam2392 · 2022-06-16T19:17:39Z

@mscheltienne FYI, see mne-tools/mne-qt-browser#117

where we can just embed as needed. Seems super simple.

adam2392 · 2022-06-27T15:02:25Z

@mscheltienne sorry I haven't gotten around to this yet, but perhaps we can do a pair session this week on getting this up and running?

I pushed up some fixes to at least let it compile.

mscheltienne · 2022-06-27T19:44:53Z

Same it was on my mind for a while.. but I was lacking the time last week. I can work on this on Wednesday or Thursday.
We can do a pair session if you want.

mscheltienne · 2022-06-28T17:33:26Z

My week filled up very quickly.. I'm sorry but I don't think I can work on it this week, it's all yours. I can have a look this weekend maybe.

…t button

mne_icalabel/gui/_label_components.py

adam2392 · 2022-07-25T20:40:52Z

Oh nice! That works, except for some OS/Python combinations it seems. I'm guessing that setting up the GUI to be robust across CI testing is non-trivial...

Does MNE-Python only test GUI on certain OS/Python combos?

larsoner · 2022-07-26T01:17:38Z

We run GUI tests on all OSes and Python versions. On Linux you need xvfb but on Windows and macOS you only need something extra if you need OpenGL support. Regular windowing and matplotlib should be fine on those OSes without any issues I think

adam2392 · 2022-07-26T01:33:47Z

I still end up getting this weird visual output. If I run things with the entrypoint, then I get this resulting weird visual.

Interestingly, it shows up this way on @ayoun25 Windows machine too, so I wonder if this is a version issue?...

adam2392 · 2022-07-26T01:34:26Z

We run GUI tests on all OSes and Python versions. On Linux you need xvfb but on Windows and macOS you only need something extra if you need OpenGL support. Regular windowing and matplotlib should be fine on those OSes without any issues I think

Swell! I think now the CI is working! black magic...

adam2392 · 2022-07-27T16:21:25Z

Notes from today:

seems like the plot_sources is plotting twice (so first off set show=False where needed), which is fixed if we explicitly instantiate the widget.
however, then the button clicking is not registering

mscheltienne · 2022-07-27T17:09:47Z

I did not have as much time to dig in as I was hopping, but the issue is that every time you click on a new IC to display, a new browser is opened and the old one is not deleted. It actually stays.. behind.
I tried different ideas with little success:

layout.removeItem()
layout.removeWidget()
browser.close()
browser.closeEvent()

This issue will be gone once we can open an initial browser and swap the instance displayed via a method supported by the browser, but for now, I still did not find how to delete/close the old browsers. @marsipu by any chance, do you know how from the top of your head?

I'll keep looking.

mscheltienne · 2022-07-27T17:11:59Z

Also, this is not macOS specific, even on my windows PC it does appear (the one I used for the quick screen recording), once you start resizing the window. So I think you are right @adam2392, it showed up on your computer and not on mine because of the resolution/monitor.

mscheltienne · 2022-07-28T11:32:14Z

@adam2392 Give it a shot, I think it's working correctly on my mac.
My hypothesis is that the C++ reference was not removed, thus the Widget was not removed even if it was detached from the layout. Setting the parent to None when replacing the browser seems to do the trick:

timeSeries_widget = self.ica.plot_sources(self.inst, picks=[self.selected_component])
self._central_widget.layout().replaceWidget(self._timeSeries_widget, timeSeries_widget)
self._timeSeries_widget.setParent(None)
self._timeSeries_widget = timeSeries_widget

adam2392 · 2022-07-28T13:27:01Z

@mscheltienne nice! Now I get this. I noticed there is no way of getting rid of the plot for the EOG channels (see https://github.com/mne-tools/mne-python/blob/9e4a0b492299d3638203e2e6d2264ea445b13ac0/mne/viz/ica.py#L998-L1006). Do we want to keep it in there? If not, we prolly need to PR the plot_sources function.

If so, I think we can merge this!

mscheltienne · 2022-07-28T13:31:54Z

mne_icalabel/gui/_label_components.py

+        set_browser_backend("qt")  # force MNE to use the QT Browser
+
+        # keep an internal pointer to the instance and to the ICA
+        self._inst = inst


@adam2392 We could drop the EOG / ECG channels here. We don't need them anyway for this GUI.

But this would modify the outside instance in place. Perhaps we leave it for now? It is kind of nice if you have it to compare against time-series traces that look "like it".

As you want, we can always do a copy before dropping the channels.
It's True that it is a nice piece of information to have to compare against the IC's time-series. The main issue IMO is the size of that browser and especially of the scrollbar/statusbar.

Let's just merge this for now for the sake of giving Aaron something to work with. We can modify as we go.

Sounds good!

alexrockhill · 2022-07-28T14:15:37Z

This is a bit longer-term but I think it's worth cross referencing to mne-tools/mne-python#10913 for the new backend abstraction that I wrote in case that makes things easier down the road with the abstraction making things cleaner and or adding notebook support. Looks great by the way!

adam2392 · 2022-07-28T14:20:56Z

@alexrockhill thanks! That's a big PR.

What's a good summary? :p

Is it that we can create the GUIs using the abstract interface? It buys us automatically notebook support?

alexrockhill · 2022-07-28T15:22:40Z

@alexrockhill thanks! That's a big PR.

What's a good summary? :p

Is it that we can create the GUIs using the abstract interface? It buys us automatically notebook support?

The summary is that it is free notebook support and implements all the widgets in a way that looks similar between Qt and notebook with the standard object-oriented design (e.g. HBox, Buttons etc.).

adam2392 added 3 commits May 31, 2022 10:04

Initial commit to show skeleton

306b53e

Some more components

3ae5dcd

Add draft gui

7fed4cd

Adding the GUI with basic Python API

1a6df47

adam2392 requested a review from mscheltienne June 15, 2022 01:02

Adding updated widget

cd0e947

mscheltienne reviewed Jun 15, 2022

View reviewed changes

mscheltienne added 3 commits June 15, 2022 13:27

draft_2

77426c1

update logic

eac7cc6

some typos

88ab828

adam2392 added 2 commits June 27, 2022 10:47

Adding testing script

cb05faa

Adding updates to make things compile

6a10148

mscheltienne added 8 commits July 6, 2022 14:26

remove old version

0b4a6ed

plot psd and topomap

a3af1d8

change dpi

d5bd516

resize interface

d9d912a

fix style

65a5beb

add entry-point to run the GUI and app.exec() to start the Qt event loop

8dfdb8e

fix style

630a49d

add saving of the selected labels to self.saved_labels and add a rese…

59d23a5

…t button

mscheltienne reviewed Jul 13, 2022

View reviewed changes

mne_icalabel/gui/_label_components.py Outdated Show resolved Hide resolved

adam2392 added 2 commits July 18, 2022 16:02

Adding updated api TO SAVE components to ICA isntance.

cf37d9a

Adding test file for TODO

fb6fb47

adam2392 mentioned this pull request Jul 18, 2022

Refactor ICA Component Labeling GUI Based on MNE-Python API #72

Open

Merge branch 'main' into gui

100c8f2

adam2392 added 2 commits July 25, 2022 16:07

Fix this

9405cd9

Fix unit test

ffc3be2

Add version restriction

8459966

Try again

0917b64

delete old browsers

f6f8794

adam2392 added 2 commits July 28, 2022 09:23

Merge

7a470f0

Merge branch 'gui' of https://github.com/adam2392/mne-icalabel into gui

487ee31

mscheltienne reviewed Jul 28, 2022

View reviewed changes

adam2392 mentioned this pull request Jul 28, 2022

Add utility function for porting from MNE-Python ICA labels/labels_score data structure to internals #81

Open

adam2392 added 2 commits July 28, 2022 09:56

Not block

5928195

Try again

a421081

adam2392 added 2 commits July 28, 2022 10:38

Try again

3b9e020

Try again

e9a7c5b

adam2392 added 2 commits July 28, 2022 23:35

Try again

5c86ae0

Try again

b3e2f1f

adam2392 merged commit 2ddc063 into mne-tools:main Jul 29, 2022

adam2392 deleted the gui branch July 29, 2022 17:55

adam2392 mentioned this pull request Jul 29, 2022

API and GUI: Creating a highly informative example/jupyter notebook for IC labeling #12

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ENH] Draft GUI for labeling components #66

[ENH] Draft GUI for labeling components #66

adam2392 commented Jun 15, 2022 •

edited

Loading

codecov bot commented Jun 15, 2022 •

edited

Loading

mscheltienne left a comment

mscheltienne Jun 15, 2022

adam2392 commented Jun 16, 2022

adam2392 commented Jun 27, 2022

mscheltienne commented Jun 27, 2022

mscheltienne commented Jun 28, 2022

adam2392 commented Jul 25, 2022

larsoner commented Jul 26, 2022

adam2392 commented Jul 26, 2022

adam2392 commented Jul 26, 2022

adam2392 commented Jul 27, 2022

mscheltienne commented Jul 27, 2022

mscheltienne commented Jul 27, 2022

mscheltienne commented Jul 28, 2022 •

edited

Loading

adam2392 commented Jul 28, 2022 •

edited

Loading

mscheltienne Jul 28, 2022

adam2392 Jul 28, 2022

mscheltienne Jul 28, 2022

adam2392 Jul 28, 2022

mscheltienne Jul 28, 2022

alexrockhill commented Jul 28, 2022

adam2392 commented Jul 28, 2022

alexrockhill commented Jul 28, 2022

[ENH] Draft GUI for labeling components #66

[ENH] Draft GUI for labeling components #66

Conversation

adam2392 commented Jun 15, 2022 • edited Loading

PR Description

Details of the GUI

Merge checklist

codecov bot commented Jun 15, 2022 • edited Loading

Codecov Report

mscheltienne left a comment

Choose a reason for hiding this comment

mscheltienne Jun 15, 2022

Choose a reason for hiding this comment

adam2392 commented Jun 16, 2022

adam2392 commented Jun 27, 2022

mscheltienne commented Jun 27, 2022

mscheltienne commented Jun 28, 2022

adam2392 commented Jul 25, 2022

larsoner commented Jul 26, 2022

adam2392 commented Jul 26, 2022

adam2392 commented Jul 26, 2022

adam2392 commented Jul 27, 2022

mscheltienne commented Jul 27, 2022

mscheltienne commented Jul 27, 2022

mscheltienne commented Jul 28, 2022 • edited Loading

adam2392 commented Jul 28, 2022 • edited Loading

mscheltienne Jul 28, 2022

Choose a reason for hiding this comment

adam2392 Jul 28, 2022

Choose a reason for hiding this comment

mscheltienne Jul 28, 2022

Choose a reason for hiding this comment

adam2392 Jul 28, 2022

Choose a reason for hiding this comment

mscheltienne Jul 28, 2022

Choose a reason for hiding this comment

alexrockhill commented Jul 28, 2022

adam2392 commented Jul 28, 2022

alexrockhill commented Jul 28, 2022

adam2392 commented Jun 15, 2022 •

edited

Loading

codecov bot commented Jun 15, 2022 •

edited

Loading

mscheltienne commented Jul 28, 2022 •

edited

Loading

adam2392 commented Jul 28, 2022 •

edited

Loading