nipy / PySurfer

Cortical neuroimaging visualization in Python
https://pysurfer.github.io/
BSD 3-Clause "New" or "Revised" License
240 stars 97 forks source link

ENH: Colormap argument for `add_overlay`. #95

Closed arokem closed 10 years ago

arokem commented 10 years ago

Added as a kwarg with "YlOrRd" as the default (which was the only possible behavior previously).

larsoner commented 10 years ago

LGTM. @mwaskom you were most recently changing the colormap code, WDYT?

larsoner commented 10 years ago

(Ignore the Travis build error issue for now, we're in the midst of adding Travis support.)

mwaskom commented 10 years ago

It's more complicated than this as an overlay has both positive and negative components, with different colormaps (PuBu is used be default for the negative map).

We also should think about how to handle reversals; technically the positive overlays get mapped with "YlOrRd_r", but mayavi doesn't understand that so the reversal logic is handled by changing the colorbar object and not in terms of the palette values.

With the colormap additions from #82 PySurfer can handle that logic itself, but it's worth thinking about what should be considered a reversal -- should the positive colormap silently reverse colormaps so that ColorBrewer palettes lighten at higher values? Or should we take the more explicit/less obvious step of making "YlOrRd_r" the default?

arokem commented 10 years ago

OK. Just to explain why I did this: the issue I had is that the functionality of add_data and add_overlay is slightly different, but one is more flexibly customizable than the other (e.g. colormap argument).

For example, I needed to overlay data from an mgh file that only has data from some vertices. This works very nicely with add_overlay, but with add_data it covers also the no-data vertices with a value of 0 (?).

I might be misusing add_data ...

On Wed, Feb 5, 2014 at 10:51 AM, Michael Waskom notifications@github.comwrote:

It's more complicated than this as an overlay has both positive and negative components, with different colormaps (PuBu is used be default for the negative map).

We also should think about how to handle reversals; technically the positive overlays get mapped with "YlOrRd_r", but mayavi doesn't understand that so the reversal logic is handled by changing the colorbar object and not in terms of the palette values.

With the colormap additions from #82https://github.com/nipy/PySurfer/pull/82PySurfer can handle that logic itself, but it's worth thinking about what should be considered a reversal -- should the positive colormap silently reverse colormaps so that ColorBrewer palettes lighten at higher values? Or should we take the more explicit/less obvious step of making "YlOrRd_r" the default?

— Reply to this email directly or view it on GitHubhttps://github.com/nipy/PySurfer/pull/95#issuecomment-34223574 .

mwaskom commented 10 years ago

add_data has a threshold= keyword argument that you should be able to use to control that. Just set the non-data vertices to some value that's lower than any of the data values and set the threshold in between them.

mwaskom commented 10 years ago

You can also change the overlay colormaps post-hoc, like in this example.

I'm not opposed to making the overlay colormaps configurable, it will just require figuring out a few things about the right way to do it. Personally I use add_data for most of my applications, though, and it should be flexible enough to do what you want.

arokem commented 10 years ago

OK - I will try playing with it some more. You can close this PR, if you think it's not useful, and would just require a lot of redesign around it.

On Fri, Feb 7, 2014 at 10:32 AM, Michael Waskom notifications@github.comwrote:

You can also change the overlay colormaps post-hoc, like in this examplehttp://pysurfer.github.io/examples/plot_fmri_conjunction.html .

I'm not opposed to making the overlay colormaps configurable, it will just require figuring out a few things about the right way to do it. Personally I use add_data for most of my applications, though, and it should be flexible enough to do what you want.

— Reply to this email directly or view it on GitHubhttps://github.com/nipy/PySurfer/pull/95#issuecomment-34484275 .

mwaskom commented 10 years ago

Closing for now as I don't have the mental bandwidth to decide how this should work, but I do get cyclical itches to make colormaps work better for everyone, so it might happen in the future.