Improve the Bad Channels picker dialog box

[chrismullins]

From the associated PR #4:

Currently, the channel picker dialog does a good job of allowing the user to
choose bad channels, but has a few shortcomings:

(1) It's difficult to see, at a glance, which channels have already been marked
as bad.

(2) When the user begins clicking on new channels to mark as bad, any
previously marked channels become unselected. Thus, making sure you've selected
everything you want requires careful scrolling.

This change introduces a few new files (including a .ui file from QtDesigner and generated .py from the pyuic5 tool) and @cbrnr might have preferences about how this sort of thing is organized/implemented. Since this might entail multiple PR's, an issue might be a better place for discussion.

Please feel free to review my code and/or let me know your thoughts and ideas.

[cbrnr]

You're right, the channel picker definitely needs some improvement. I've avoided the Designer so far because I think that simple dialogs are easier to write in code (and they don't require extra files and tools) - but of course this is purely subjective.

That said, I'm not sure the dual list approach is the best solution. I was thinking that we should probably add a more powerful "Edit channels" dialog that brings together all channel-related functionality, such as renaming channels, marking/unmarking channels as bad, and maybe also dropping channels (which currently has its own separate dialog). There could also be a color picker for each channel (important for visualizations). We'd also need to display the channel types somehow. A tree view which has channel types (EEG, MEG, STIM, ...) as top-level nodes would be a suitable way to represent this information. Then we could have checkboxes to select channel types and/or individual channels (as bad channels), and channel names could be editable.

Issue (2) you mention could be more easily solved by changing the selection mode to QAbstractItemView::MultiSelection. Issue (1) can probably also be solved by making the dialog window a bit larger so that more channels are visible at a time.

[cbrnr]

On the other hand, clicking Ctrl (or Cmd on a Mac) adds a new selection even with the current selection mode, which is the default behavior on most OSs.

[chrismullins]

Good ideas, I especially like the concept of a "Channel Editor" for all channel properties. Happy to work on this, if you want to create an issue and assign it to me!

It could enable:

• Renaming channels
• Marking/unmarking as "bad"
• Dropping channels
• Colormapping certain channels to override defaults -- I'm hesitant about this one because (1) there's no direct mapping to a channel property meaning we'd have to keep track of it ourselves and (2) it could blur the distinction between the "Model" and the "View." But this might not be such a problem.
• Viewing/setting channel type

Issue (2) you mention could be more easily solved by changing the selection mode to QAbstractItemView::MultiSelection. Issue (1) can probably also be solved by making the dialog window a bit larger so that more channels are visible at a time.

I see your point, hadn't thought of that. Simpler is probably better right now.
Probably the best UI element to use here is a checkbox in the "Bad" column, within a channel editor -- which then colors that channel's row red within the dialog or something.

The most important thing is to present a Single Point Of Truth within the editor. (eg. It should be obvious to the user whether she's appending to a list or overwriting it by accepting the current state of the dialog.) And so it makes sense to present all the properties of all the channels in one place.

[cbrnr]

Yes, maybe we should skip the color mapping for now. Also, I'm thinking that dropping channels should probably still go into a separate dialog because this modifies the actual data, whereas renaming and marking channels as bad just edits the metadata.

What do you think of this layout:

Here, the columns can be sorted by clicking on the headers. Labels can be edited by clicking on them. We might enable editing for the type as well. If MNE has predefined types, then we present a dropdown list to choose from, or if there are no restriction on channel types we can just have line edits. The Bad column should contain checkboxes.

The only gripe I have about this design is that channels need to be marked as bad individually. We can't just have a selection turn into bad channels, can we?

[chrismullins]

I think the types of channels are defined in the docstrings of mne.channels.channels.channel_type function. QComboBox seems like the best option, is this what you had in mind?

I don't see why we couldn't have a checkbox as an item in the table, and then a button that says "Mark Selected as Bad". Maybe a button with "Edit Selected" opens an additional dialog on the selected rows, so you can do other stuff like change the type of all of them at once, mark them all as bad, etc, or something like that.

[cbrnr]

I think the types of channels are defined in the docstrings of mne.channels.channels.channel_type function.

Can we get a list of supported types from MNE?

QComboBox seems like the best option, is this what you had in mind?

Basically yes, but can the comboboxes styled so that they look like plain text, and if you click on it a dropdown list appears with the possible choices? Otherwise they generate too much visual noise.

I don't see why we couldn't have a checkbox as an item in the table, and then a button that says "Mark Selected as Bad".

Yes, why not. What if the selection only contains bad channels and you would like to unmark them? We could name the button "Toggle bad" or something (do you have a better suggestion?), but what if the selection contains both bad and good channels? Do we also just toggle them? Also, I'd get rid of the checkboxes in each row then because there shouldn't be two ways to mark channels as bad (just put some text e.g. "bad" if a channel is marked as bad and nothing otherwise).

Maybe a button with "Edit Selected" opens an additional dialog on the selected rows, so you can do other stuff like change the type of all of them at once, mark them all as bad, etc, or something like that.

Or let's just have a second button in addition to the "toggle bad" button - called e.g. "change type", which pops up a small dialog with a combobox or a list widget showing available types. Then we could get rid of the comboboxes in the main table widget.

[chrismullins]

Can we get a list of supported types from MNE?

I wish, but I sort of doubt it. If you look a little further into that function you can see it just checks to see if the constants line up with certain values in the FIFF class in constants.py which appears to define every constant used anywhere in MNE.

If you do want per-channel checkboxes, the behavior could be identical logic to gmail's interface.

Otherwise, we could just have the user select whichever rows and then have a "Toggle Bad on Selected" button. We could, in addition, have the dialog you mention pop up (second button called "Advanced") for more complex manipulations such as "Change Channel Type," "Pick Channel(s)", etc. I think I'm partial to this, as long as we can make each row small enough to show at least half the channels on the screen at one time, to minimize scrolling.

I'll start mocking this up, and once we can use it it'll be easier to talk about.

[chrismullins]

See #8: if you pull down this branch and load in some data, you can see the first design iteration I've come up with. Please leave suggestions/likes and dislikes.

Load in some data, and open the Edit->Set Channel Info... dialog box. From there you can select certain channels and edit Channel Type/Bads individually, or open the "Edit Selected" dialog which allows bulk editing of all the selected ones at once. Any other categories or logic you can think of that we need in the first version of this?

Don't look too closely at the actual code, it's not very clean yet ;)

[cbrnr]

Thanks for this mockup. I don't think that a second dialog where you can edit multiple channels is necessary though. One possible intuitive solution is that if more than one channel is selected (and BTW the selection should extend over the whole row and not just a particular column), changes made to any item (i.e. mark/unmark as bad or set channel type) are propagated to all selected rows. What do you think?

[cbrnr]

Also, here's the stylesheet I'd like to apply to the QComboBox items - this looks really neat:

/* remove borders and padding */
QComboBox {
    border: 0px;
    padding: 0px;
}

/* remove dropdown arrow */
QComboBox::drop-down {
    width: 0px;
    border-width: 0px;
}

[cbrnr]

[chrismullins]

My original thought was to have that menu in case later we needed to do something more complicated with them -- but the more I look at it the more I agree with you (and the YAGNI principle). I'll replace that with better widget interaction logic you mention.

the selection should extend over the whole row and not just a particular column)

You're right, this will be fixed like this:

self.ui.tableWidget.setSelectionBehavior(QAbstractItemView.SelectRows)

I like your style sheet! Definitely going to use that, thanks for sharing. The only problem is that I think it's overwriting whichever property tells it which color the QComboBox background should be when it's "selected" (not clicked for editing, but highlighted as if the name was clicked).

So I end up with it looking like this:

I messed around with the various stylesheet properties but couldn't find the one to set for this to be fixed. Do you know what I might be missing?

Anyway I'll make this one now and submit another PR shortly. Thanks for the input!

[chrismullins]

Checkout #9 and let me know what you think.

Ideas I have going forward (but which should not block this issue):

• How about setting a different stylesheet for bad channels? A red tint across the whole row might be nice and obvious.
• An "undo" button.
• A "plot channels" button, for if you only want to look at a subset of channels -- you could just highlight them and plot them all on one window. Especially useful in case you forget which one had all that line noise in it, double check before marking it as bad.