Allow custom colormap objects or registered names

[mwaskom]

This expands the flexibility of our what we accept as a colormap a bit to matplotlib colormap objects or the name of a custom colormap that has been registered with matplotlib.

This does not fix a weird behavior in pysurfer where lists are assumed to have colors in [0-1] and arrays are assumed to have colors in [0-255]. I have always thought that is probably a bug, but fixing it might break things for people who are dealing with it.

[larsoner]

Flake error:

https://travis-ci.org/nipy/PySurfer/jobs/251319341#L945

Worth updating some example?

[mwaskom]

Unused import removed.

I'm not sure there's a good example script because for a colormap with a small number of colors we already accept a list or array -- defining a colormap object is overkill. The primary usecase would be when you want to use a colormap object that some other package (namely, seaborn, where I just added some colormaps that will be useful in pysurfer) has registered with matplotlib. But we don't want the doc build to depend on seaborn.

[larsoner]

But we don't want the doc build to depend on seaborn.

Why not? It's easy enough to install (pure Python) and I think it would be cool to show off such functionality.

[mwaskom]

Actually on second thought, why not just package the new colormaps with PySurfer and register them with matplotlib (on pysurfer import I guess?).

Here they are:

[mwaskom]

Oh, right, the import of matplotlib.cm was actually functional despite not being directly referenced.

[larsoner]

Actually on second thought, why not just package the new colormaps with PySurfer and register them with matplotlib (on pysurfer import I guess?).

I like the Seaborn way for a two reasons. One, it's deduplicated /more future compatible in that as more are added to Seaborn we don't also need to add them here. Two, it shows how to integrate with another useful package.

[mwaskom]

Well to be clear, the existing code in this PR will be future compatible in that any colormaps that are added to seaborn (or elsewhere) in the future and registered with matplotlib will "work" in pysurfer without packaging them on our side. But given that it happens behind the scenes, I worry that it's a confusing example -- giving the name of a certain colormap will only work when seaborn has been imported, even though seaborn is not used directly.

Also given that until 0.8 importing seaborn changed how matplotlib plots looked, there is reason not to necessarily want to have it imported beyond script cruft.

[larsoner]

Well to be clear, the existing code in this PR will be future compatible in that any colormaps that are added to seaborn (or elsewhere) in the future and registered with matplotlib will "work" in pysurfer without packaging them on our side. But given that it happens behind the scenes, I worry that it's a confusing example -- giving the name of a certain colormap will only work when seaborn has been imported, even though seaborn is not used directly.

I thought that picking up on such names was the point of this PR, though, no? If so, this is an issue of documentation more than anything else. We can say something about how we can integrate with seaborn, however it is done: import it, call whatever is necessary, and PySurfer is now smart enough to use it.

Also given that until 0.8 importing seaborn changed how matplotlib plots looked, there is reason not to necessarily want to have it imported beyond script cruft.

The idea is to have an example showing people how to use it, optionally, if they want to. It's also very easy to update such pure python dependencies. So I'm not so convinced this is a blocker.

If instead duplicate them here, do we also need PySurfer to register colormaps on import, too, then? Or we'll need to add some function to do it (further duplicating the Seaborn functionality)? Or will it catch these special names already without any registering?

[mwaskom]

If instead duplicate them here, do we also need PySurfer to register colormaps on import, too, then? Or we'll need to add some function to do it (further duplicating the Seaborn functionality)? Or will it catch these special names already without any registering?

There are three options:

1) don't register the colormaps, but tells users to pass the colormap object, like cmap=surfer.cm.icefire. 2) register the colormaps by name on pysurfer import so that cmap="icefire" will "just work". 3) special-case the names of the "pysurfer" colormaps in create_color_lut so that cmap="icefire" will work in the context of pysurfer, but elsewhere in matplotlib.

To support options 1 and 2 you'd need (a) the existing code in this PR and (b) to copy this module from seaborn into pysurfer. (To support option 1 but not 2, you'd want to remove the calls to register_cmap at the end of the module). Option 3 would need a little additional work.

[larsoner]

1) isn't very good for the user, 2) is too invasive / magical IMO, but 3) is okay. I still prefer telling people to use (and update if necessary) seaborn to get this functionality, but can live with 3) (so long as you promise to keep PySurfer up to date with seaborn updates :) )

[mwaskom]

2) is too invasive / magical IMO

Well, to be fair, that's how it works in seaborn, so if you want to avoid that behavior, best to avoid depending on seaborn.

[mwaskom]

OK the latest commit implements option 3.

[larsoner]

LGTM, +1 for merge once CIs come back happy

[mwaskom]

Shall I hit the big green button?

[larsoner]

Thanks @mwaskom