Cubehelix #13
Cubehelix #13
Conversation
cubehelix.py was taken from commit 42eb722d455fc393a094291cbedad8d4f9588272 of http://github.com/jradavenport/cubehelix by @jradavenport Added a module docstring and license
The original cubehelix function is now a static method of Cubehelix so that it can provide a color list to the Palette baseclass.
Changed some parameter names to be more expressive and also use
underscores rather than camel case.
I decided to keep both the {start_hue, end_hue} and {start, rotation}
parameter pairs. start_hue and end_hue will override start and rotation
if their values are both set.
Add general code improvements to the cubehelix math function. Renamed some variables to be more expressive: - angle -> phi - fract -> lambda This change also disambiguates when fract was used to refer to just lambda and lambda^gamma. Made the choice between a scalar saturation and a vector of saturations a bit cleaner. Numpy magic: - Use np.dot to do equation 2. This means the function returns a (n, 3) numpy array. The Palettable class seems happy with this though. Also apply scaling to 255 right in the rgb equation - Use np.clip for the clipping Question of phi equation: The equation for phi in Green 2011 does not have an extra `+1` but the Fortran code does, as does the original cubehelix.py I'm leaving out the +1 to keep to the original equation, but worth investigating. In practise I see no difference!
Instead of hard-coding colours in, like for the Brewer and Tableau palettes, here it makes sense to hard code just the parameter dictionaries. I'm also using an OrderedDict rather than having two separate name and value arrays to maintain order while also keeping data together. get_map() instantiates Cubehelix from these parameters. It instantiates 256-color palettes, which is appropriate for continuous color maps. Added several prebuilt maps from @jradavenport's original implementation docs and from http://nbviewer.ipython.org/gist/anonymous/a4fa0adb08f9e9ea4f94
This solves the hex encoding issue and should also fix Python 3 issues.
1 similar comment
|
@jiffyclub Looks like there are two test issues here:
|
|
You can see in palette.py how I'm protecting matplotlib imports. There's enough numpy usage in there that I don't necessarily want to make you replace it all with pure python. What if the data for all the default palettes was pre-computed and stored in the file so that people only needed numpy when creating custom versions of cubehelix? It looks like the default palettes are defined by 256 RGB points, which is probably more than strictly necessary. matplotlib does linear interpolation between the points you give it, so if you generated even 16 RGB points for the default palettes I bet they'd look decent (though that'd require some testing). |
|
I can take care of adding a doc page before the next release, but do feel free to add a notebook to the demo/ directory. The colors as 2D array thing works because ndarrays are sufficiently list like when you iterate over them. You get a 1D row of length three on each iteration, which is what the code is expecting. All the looping looks like We'll need to fix the hex colors coming out the the L's, that's not right. There must be long integers: We can fix that by coercing to |
| # Test consistency | ||
| known_colors = ['#000000', '#237433', '#DB8ACB', '#FFFFFF'] | ||
| palette = cubehelix.Cubehelix(n=4) | ||
| for c1, c2 in zip(palette.hex_colors, known_colors): |
There was a problem hiding this comment.
Here and below you can probably directly compare these lists, pytest is great about giving you useful output about mismatched list items.
|
Here's an example of how you don't need a large number of RGB points to make these palettes:
So I think we could pre-generate the default cubehelix palettes and store the numbers in there so folks can access them with a pure Python install of palettable. Maybe add a class method or something for creating a |
|
Yeah! I'll push something soon with the class method and pre-baked RGB lists. I'm just at a meeting right now at github of all places. |
The default palettes are changed from 256 colors to 16, and the RGB values are baked into the module. Cubehelix now takes a sequence of RGB colors, like the other Palettable subclasses. The `make` class method does the cubehelix algorithmic calculations to generate a Cubehelix instance. Also changed from OrderedDict to regular dictionaries with a palette_names list for ordering. This fixes Python 2.6 compatibility.
Introduce a HAVE_NPY module variable to indicate the ability to import numpy for cubehelix and raise an except if Cubehelix.make() is called without numpy in the environment. Also skip tests where numpy is needed but not available.
1 similar comment
|
Looks great! I saw you went with a class method for making the custom palettes. That was the only way I could think to make it work as well. An alternative solution would be to keep the Cubehelix class as you had it originally but have the default ones be Palette instances instead. But then they aren't Cubehelix instances. I'm happy with the Cubehelix.make class method if you are. |
|
I think the class method makes total sense. Better to keep the API uniform and be explicit about generating the palette rather than using one Do you have any stylistic requests? I'll fix the test coverage gaps. |
| """ | ||
| Returns a dictionary of all Cubehelix palettes, including reversed ones. | ||
|
|
||
| These default palettes are rendered with 256 colours, making them useful |
There was a problem hiding this comment.
This is now 16 colors, but it's not really important to bring up since any sequential or diverging palette from palettable is suitable for continuous maps because matplotlib takes care of interpolation.
|
Looks great! Such PEP8, very wow. 😸 I think the last thing to make sure of is that we aren't ending up with L's in the hex color strings. |
Covers print_maps() and KeyError raised by get_map.
A failure in the index lookup actually raises a ValueError, so now we properly re-raise it as a KeyError to conform to the API.
All the pre-built maps use 16 colors, and I'm not sure it makes sense to build maps with different numbers of colors. Thus get_map() has no explicit n keyword arg. But so that it won't fail I simply print out the the user arguments there were supplied but unused. This will let the API gracefully change in the future.
|
I think I've resolved the last few details:
|
Accurate docstrings for both the class __init__ and for the make classmethod now.
|
One thing I just remembered is that I've started putting the number of defined colors at the end of the palette names. So e.g. in the tableau module there are palettes named like BlueRed_6 and BlueRed_12. Here those would all be _16, but for consistency I think it should be there. I put that suffix there so people accessing the palettes via tab completion can quickly tell how many defined colors there are. |
Since the number of colors will be specified in a maps's name, it doesn't make sense to support this functionality.
The pre-made cubehelix maps now conform to the palettable standard for
{name}_{n}[_r] naming._
Also update demo notebook to reflect new names and number of colors in a
pre-make color map.
|
Looking good! Ready to go? |
|
@jiffyclub I think so! Do you want me to squash everything into one commit? Also, I was trying to make the docs with cubehelix but urubu was getting links wrong (even to the existing sub pages). So maybe I'll leave docs off of this PR after all. |
|
Urubu can make it hard to preview things locally because they have to served from a http://host/palettable/ URL. I had to make the tservice tool specifically for previewing these docs locally, e.g. with I don't care one way or the other about squashing commits. If you want to do any cleanup go for it, but I'm happy to merge whenever. |
|
Okay, give me a sec and I'll see if I can ship some docs with it. |
The cubehelix doc page discusses both the pre-made palettes and has section showing how to make cubehelix palettes.
|
I've put together a doc page for cubehelix. It includes both previews and an API reference section for making palettes. I don't entirely like just listing the arguments the way I did, but it works. Maybe laying out the API via a table would be better.
|
?
|
A table would work, but I don't want to muck around with a table in Markdown. Honestly I was just going to put in the raw output of |
Port of @jradavenport's cubehelix module Original at https://github.com/jradavenport/cubehelix.
|
Thanks for your help with this! |
|
Thank you! |
This PR implements cubehelix color maps in palettable by porting @jradavenport's
cubehelix.py. I put a demo notebook up as a gist here. I think it's sweet.I've kept the API for
palettable.cubehelixas similar as possible to the other palettes. Basically, theCubehelix(Palette)class computes colors for the color map.get_map()is a factory that takes pre-defined parameter sets and createsCubehelixinstances. The user is also encouraged to build their own maps directly.I've spruced up the cubehelix algorithm implementation a bit. Some parameters are renamed for clarity. I also use
numpy.dot()to build the entire RGB color vector in one statement. Anyways, it's a bit more Pythonic and a little less... Fortranic?I also noticed a discrepancy between D.A. Green's formula for
phiand the Fortran code (and the original Python implementation); there's an issue of adding an extra+1. I've stayed true to the formula in the paper, although I don't actually notice any difference in the outputs.Some issues I see:
palettable.cubehelix.(n, 3)numpy array rather than a list of 3-tuples. Surprisingly, this works. The one problem is that hex colors contain an'L', e.g.,'#23L74L33L'. What's up with this? Alongtype? Is this ok?demo/if you think that's a good idea.Let me know what you think!