Selecting ICA components in `ica.plot_components` doesn't work anymore

[cbrnr]

If I click on the title of a component in the dialog produced by ica.plot_components(inst=raw), I get the following error:

  File "/usr/local/lib/python3.6/site-packages/matplotlib/cbook/__init__.py", line 389, in process
    proxy(*args, **kwargs)
  File "/usr/local/lib/python3.6/site-packages/matplotlib/cbook/__init__.py", line 227, in __call__
    return mtd(*args, **kwargs)
  File "/usr/local/lib/python3.6/site-packages/mne/viz/topomap.py", line 1006, in onclick
    label = event.inaxes.get_label()
AttributeError: 'NoneType' object has no attribute 'get_label'

Any idea why the label is gone and how to retrieve it (actually, event.inaxes is None)?

[mmagnuski]

I'll take a look in the evening, which matplotlib version do you have?

[cbrnr]

Thanks! I have the latest Matplotlib (2.1.0).

[mmagnuski]

What is your expectation for what should happen when you click the title? I can replicate your problem but if you want to open component properties then click on the topo, not the title. Then this would be an easy fix.

[cbrnr]

Clicking on the title should mark/unmark a component as bad (i.e. include/exclude in ica.exclude).

[mmagnuski]

Ah, right, that's something that you've added IIRC. Where is that set up in the code (I looked but couldn't find it)? Because the error comes from the piece of code that plots properties, not tries to change ica.exclude.

[cbrnr]

What, I added this? 😮

[mmagnuski]

Correct me if I'm wrong. :) I looked into PR history and found this related PR: #4285, but it does only change title coloring, no interactivity is added. When was the title-related interactivity added?

[cbrnr]

AFAIR I wanted to add interactivity but then we decided to quickly merge this PR with only the color change implemented. So even if I did not yet add interactivity we still need to handle this error, right? We should be able to click on the title and nothing should happen.

[mmagnuski]

Yes, that was why I mentioned that it should be an easy fix if clicking on titles doesn't have any intended functionality now. I will push a PR today. I can also add the selection by title clicking, unless you want to do it. :)

[cbrnr]

Actually, if you want to add the interactive functionality I'd be more than happy! 😄

[mmagnuski]

Ok, I'll try to do it in one PR.

[mmagnuski]

Because clicking on the title will always give you inaxes of None, the selection (changing state from excluded to included and the reverse) can be done in an easy or hard way:

• easy - right-clicking the topo changes IC state
• harder - clicking the title changes ICA state, but that would require checking the bounding boxes of titles with respect to xy pos of the mouse click.

Because of this I will split this into two PRs. First - small fix so that inaxes of None is ignored and only when we decide which of the above scenario we want I'll add it in another PR.

[mmagnuski]

BTW anyone knows how to quickly find all the title text objects? In this case I can do it by inspecting all text object and seeing which have a text beginning with 'ICA' but I was wondering if there is an easier way.
Once this is done we could check all text objects with their contains method against the mouse event.

[larsoner]

Instead of inspecting all, we know there is only one we care about. So it might be easier to know which Axes instance you're interested in and check only the contains of its title text object as ax.title IIRC.

[cbrnr]

Because clicking on the title will always give you inaxes of None

Did this change recently? AFAIR, you could click on the title and nothing happened.

From a UX perspective, the harder option (clicking on the title) would be preferable. In plot_raw, you can click on the y-axis channel labels to select/deselect a channel - is this something similar you could use?