MRG: Add support for vector data in `add_data`

[larsoner]

Eventually this will let us add vector data like (n_vertices, 3, n_time), which is useful for electrophysiological estimates (e.g., MEG inverse solutions). Right now it doesn't look so good but it at least does not throw an error.

Code adapted from the MNE PR by @wmvanvliet.

Todo:

• [x] do not upsample the vectors to the high-res mesh
• [x] check set_time_index
• [x] fix scale_data_colormap
• [x] split from add_data to add_vectors
• [x] add optional nn=None argument
• [x] fix TimeViewer / set_data_smoothing_steps
• [x] check data addition/removal
• [x] check time_viewer
• [x] deal with any outstanding XXX's in code
• [x] add/test brain surface transparency (glass brain)
• [x] make sure everything is pretty
• [x] add tests

[wmvanvliet]

Awesome!

[larsoner]

Okay this is ready for review/merge from my end. AppVeyor is unhappy because I'm in the midst of reconfiguring it. Here is the example output, which is not a pretty as what @wmvanvliet has posted previously because this isn't a free-orientation solution (but we can add it later):

I also snuck in fixes for normal-setting (needed .update() calls) and set_data_smoothing_steps, which are both broken in master. I also enabled backface culling for the brain surface, which seems to clean up the translucent plot quite a bit. But I can roll that back (or make it optional) if it's not agreed to be better.

[wmvanvliet]

I'll test it first this weekend when I get a chance

[wmvanvliet]

[cancel my last comment, it's working fine when I scale the colormap properly]

[larsoner]

So +1 for merge or do you need to look or test more?

[mwaskom]

I don't know anything about the kind of data you're working with here, but the new translucent cortex does look very nice!

[wmvanvliet]

I want to test a little more.

[wmvanvliet]

There seems to be a problem with the transparent arrow glyphs. They are transparent to the cortex surface, but not to the data plotted on the surface... They may be a rendering order thing. I remember encountering it a while back and having to play with the ordering.

[larsoner]

Right now I don't set the alpha of the arrows to anything (so they are always 1.). Should I set their alpha to match that of the data surface instead? Maybe that is what you see?

[larsoner]

Commit pushed to split vector alpha.

@wmvanvliet does it fix your issue?

[wmvanvliet]

Setting vector_alpha < 1.0 does indeed solve the transparency issue, but I'd like my vectors to be solid, and setting vector_alpha=1.0 still shows the same bug.

[wmvanvliet]

As a hack setting vector_alpha=0.999999 seems to work fine. Strange.

[wmvanvliet]

brain.set_time() breaks the transparency of the vectors. This may be related to the bug.

[larsoner]

@wmvanvliet can you detail how to see this breakage?

[larsoner]

(and post a screenshot with the problem)

[wmvanvliet]

Running plot_vector_mne_solution.py from https://github.com/mne-tools/mne-python/pull/3842 shows gray arrows that should be transparent:

Calling brain.set_time() resets the scaling of the colormap of the arrows:

[larsoner]

Okay @wmvanvliet both problem solved, the first by forcing 1->0.99999, and the second by setting the lut and data_range after any calls to update (as it resets them). Can you see if it works for you now?

[wmvanvliet]

Beautiful! Everything works on my end now.

[wmvanvliet]

http://imgur.com/oHsI24D

[agramfort]

https://twitter.com/agramfort/status/890665952362110976 :)

[larsoner]

@mwaskom looks like the MEG folks are happy with the functionality, do you have time to look at the code to make sure it fits PySurfer properly?

[mwaskom]

Personally it feels like a lot of extra complexity is being stuffed into the poor add_data method, but I can live with it if it's important to the MEG folks.

Going forward it might be worth thinking about a path towards more focused interfaces to plotting 4D/5D data.

[larsoner]

Going forward it might be worth thinking about a path towards more focused interfaces to plotting 4D/5D data.

Yeah I agree it adds some complexity. I had initially tried adding a new add_vectors method, but it ended up introducing some redundancy. Do you think it would be cleaner that way, though?

[mwaskom]

I only gave it a small amount of thought but did see how it would be hard to avoid redundancy without completely reworking things, so I'm not overly bothered by this as a current solution.

[larsoner]

I'm not overly bothered by this as a current solution.

Nice (unintentional) pun in there, since we're talking about visualizing a minimum-norm solution / estimates for neural currents...

Any objection to merge? If not let's get it in this weekend so @wmvanvliet can move forward with the MNE improvements.

[mwaskom]

Any objection to merge?

No resistance here.

[agramfort]

No resistance on my end.

same for me

[larsoner]

@christianbrodbeck do you want to have a look? You've worked with mayavi internals a lot, and might be interested in the vector viz, too.

[larsoner]

Plotting vector data without time axis is currently somewhat complicated (e.g. setting time_label=None), might it be worth to illustrate it in the method docstring?

I think it might be YAGNI, at least to start. Once people without time axes want to start using it we can make it clearer :)

[christianbrodbeck]

I see

[larsoner]

I added the note anyway since it didn't take too much, @christianbrodbeck can you check to see if it's sufficient?

[christianbrodbeck]

Yes looks good!

[larsoner]

Okay, I'll merge once CIs are happy, thanks for the reviews -- and @wmvanvliet for doing most of the hard work with getting it working in the first place