Allow more than 2 colormaps again #493
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1468,9 +1468,9 @@ def _find_intermediate_color(lowcolor, highcolor, intermed): | |
| diff_1 = float(highcolor[1] - lowcolor[1]) | ||
| diff_2 = float(highcolor[2] - lowcolor[2]) | ||
|
|
||
| inter_colors = (lowcolor[0] + intermed * diff_0, | ||
| lowcolor[1] + intermed * diff_1, | ||
| lowcolor[2] + intermed * diff_2) | ||
| return inter_colors | ||
|
|
||
| @staticmethod | ||
|
|
@@ -1503,35 +1503,53 @@ def _unconvert_from_RGB_255(colors): | |
| return un_rgb_colors | ||
|
|
||
| @staticmethod | ||
| def _map_face2color(face, colormap, vmin, vmax): | ||
|
There was a problem hiding this comment. Is support for multiple colormaps mentioned in the docstring for trisurf anywhere? (or is it supported in a different function that calls this function?) I didn't see any mention of multiple colormaps. What's the usecase for including multiple colormaps in the call to trisurf, e.g., how would people specify which colormap goes with which datapoint? There was a problem hiding this comment. @Kully and @choldgraf did you guys resolve this one? There was a problem hiding this comment. I think I see now what @Kully was getting at, though it took a fair bit of digging for me to figure it out. I wanted to ask - in terms of interpolating between points of a list of multiple colormaps, why not use |
||
| """ | ||
| Normalize facecolor values by vmin/vmax and return rgb-color strings | ||
|
|
||
| This function takes a tuple color along with a colormap and a minimum | ||
| (vmin) and maximum (vmax) range of possible mean distances for the | ||
| given parametrized surface. It returns an rgb color based on the mean | ||
| distance between vmin and vmax | ||
|
|
||
| """ | ||
| if vmin >= vmax: | ||
| raise exceptions.PlotlyError("Incorrect relation between vmin " | ||
| "and vmax. The vmin value cannot be " | ||
| "bigger than or equal to the value " | ||
| "of vmax.") | ||
| # find the normalized distance t of a triangle face between vmin and | ||
| #vmax where the distance is normalized between 0 and 1 | ||
|
There was a problem hiding this comment. 🎎 missing a |
||
| t = (face - vmin) / float((vmax - vmin)) | ||
|
There was a problem hiding this comment. 🐄 Can you move this check below the |
||
|
|
||
| if len(colormap) <= 1: | ||
| t_color = colormap[0] | ||
|
There was a problem hiding this comment. If |
||
| color = FigureFactory._convert_to_RGB_255(t_color) | ||
| color = FigureFactory._label_rgb(color) | ||
|
There was a problem hiding this comment. 🐄 Use a different variable name here for readability. |
||
| else: | ||
| # account for colormaps with more than one color | ||
| incr = 1./(len(colormap) - 1) | ||
|
|
||
| low_color_index = int(t/incr) | ||
|
|
||
| if t == 1: | ||
| t_color = colormap[low_color_index] | ||
|
There was a problem hiding this comment. Similar to https://github.com/plotly/plotly.py/pull/493/files#r65412272. Why not just do There was a problem hiding this comment. so, On Wed, Jun 1, 2016 at 11:19 AM, Andrew [email protected] wrote:
There was a problem hiding this comment.
That gets me the last element in the list. How is that helpful here? There was a problem hiding this comment. here's where I thought it might be easy to just use There was a problem hiding this comment.
We're still doing that in another PR right? I just got Andrew's easier way of computing t_colors working. There was a problem hiding this comment. ah sure - just lemme know when this is merged. Might be a little bit of time though, because I'm heading out of town for a few days tomorrow There was a problem hiding this comment.
Okay no problem. |
||
| color = FigureFactory._convert_to_RGB_255(t_color) | ||
| color = FigureFactory._label_rgb(color) | ||
|
|
||
| else: | ||
| t_color = FigureFactory._find_intermediate_color( | ||
|
There was a problem hiding this comment. ah, so by "multiple colormaps" do you just mean "will allow 2 colormaps and assume these are the min/max ends of a spectrum, interpolating colors in between"? There was a problem hiding this comment.
I mean more than 2 colors and yes, will interpolate between all the colors linearly, first deciding which two colors it is 'in between'. I should make all this more clear in the doc, you're right. I'll do that now after the test hopefully passes There was a problem hiding this comment. Ah I see - I guess I'm having a hard time following the function. I don't see any There was a problem hiding this comment.
I thought I responded to this message. Oh well, I said that I want a user to be able to enter a sequence of n colors and this can create a divergent type of colormap, where each facecolor is placed between its closest colors in colormap based on its relative position between vmin and vmax. So you'll get a flow from one color to the next in the plot. I already updated the string docs locally and will update once the tests pass. There was a problem hiding this comment. Also, just my intuition here, I think it's better to have this handled by the colormap itself, rather than internally in a function like this. E.g., a matplotlib colormap will accept any value from 0 to 1, and will linearly interpolate according to the colormap's parameters. Is there some kind of functionality like this in plotly? (this PR is implementing something like this, but IMO it should be a more general function that is used elsewhere in the codebase and not just for trisurf) There was a problem hiding this comment.
This sounds like a good idea. Much more structured. @theengineear Can you have a quick look at this PR? It looks finished to me and I'd like to push it to master. There was a problem hiding this comment.
Thanks. I'll fix this all up. There was a problem hiding this comment. If you guys are curious, I threw together a quick proof of concept here: https://github.com/choldgraf/ecogtools/blob/master/ecogtools/colors.py Right now it depends on There was a problem hiding this comment. Actually, it's in its own repository now, plus here's an example: https://github.com/choldgraf/colorbabel/blob/master/examples/demo.ipynb |
||
| colormap[low_color_index], | ||
| colormap[low_color_index + 1], | ||
| (t - low_color_index * incr) / incr) | ||
| color = FigureFactory._convert_to_RGB_255(t_color) | ||
| color = FigureFactory._label_rgb(color) | ||
|
|
||
| return color | ||
|
|
||
| @staticmethod | ||
| def _trisurf(x, y, z, simplices, colormap=None, color_func=None, | ||
| plot_edges=False, x_edge=None, y_edge=None, z_edge=None, | ||
| facecolor=None): | ||
| """ | ||
| Refer to FigureFactory.create_trisurf() for docstring | ||
| """ | ||
|
|
@@ -1556,8 +1574,10 @@ def _trisurf(x, y, z, simplices, colormap=None, color_func=None, | |
| if len(color_func) != len(simplices): | ||
| raise ValueError("If color_func is a list/array, it must " | ||
| "be the same length as simplices.") | ||
|
|
||
| # convert all colors to rgb | ||
| for index in range(len(color_func)): | ||
| if isinstance(color_func[index], str): | ||
| if '#' in color_func[index]: | ||
| foo = FigureFactory._hex_to_rgb(color_func[index]) | ||
| color_func[index] = FigureFactory._label_rgb(foo) | ||
|
|
@@ -1581,10 +1601,16 @@ def _trisurf(x, y, z, simplices, colormap=None, color_func=None, | |
| else: | ||
| min_mean_dists = np.min(mean_dists) | ||
| max_mean_dists = np.max(mean_dists) | ||
|
|
||
| if facecolor is None: | ||
| facecolor = [] | ||
| for index in range(len(mean_dists)): | ||
| color = FigureFactory._map_face2color(mean_dists[index], | ||
| colormap, | ||
| min_mean_dists, | ||
| max_mean_dists) | ||
| facecolor.append(color) | ||
|
|
||
| # Make sure we have arrays to speed up plotting | ||
| facecolor = np.asarray(facecolor) | ||
| ii, jj, kk = simplices.T | ||
|
|
@@ -1653,10 +1679,12 @@ def create_trisurf(x, y, z, simplices, colormap=None, color_func=None, | |
| :param (array) simplices: an array of shape (ntri, 3) where ntri is | ||
| the number of triangles in the triangularization. Each row of the | ||
| array contains the indicies of the verticies of each triangle | ||
| :param (str|tuple|list) colormap: either a plotly scale name, an rgb | ||
| or hex color, a color tuple or a list of colors. An rgb color is | ||
| of the form 'rgb(x, y, z)' where x, y, z belong to the interval | ||
| [0, 255] and a color tuple is a tuple of the form (a, b, c) where | ||
| a, b and c belong to [0, 1]. If colormap is a list, it must | ||
| contain the valid color types aforementioned as its members. | ||
| :param (function|list) color_func: The parameter that determines the | ||
| coloring of the surface. Takes either a function with 3 arguments | ||
| x, y, z or a list/array of color values the same length as | ||
|
|
@@ -2931,11 +2959,13 @@ def _label_rgb(colors): | |
|
|
||
| """ | ||
| if isinstance(colors, tuple): | ||
| return ('rgb(%s, %s, %s)' % (colors[0], colors[1], colors[2])) | ||
| else: | ||
| colors_label = [] | ||
| for color in colors: | ||
| color_label = ('rgb(%s, %s, %s)' % (color[0], | ||
| color[1], | ||
| color[2])) | ||
| colors_label.append(color_label) | ||
|
|
||
| return colors_label | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the benefit gained by making it a list and looping through? IME using arrays instead of lists almost always makes things much faster.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't necessarily think it's faster or anything like that. I actually just switched it back to this because I wanted it to work with my FF converting code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if there's a benefit other than speed. E.g., if you thought the user might pass in tuples of variable shapes then it's probably better for the input to be a list or tuple or something (since arrays require consistency across dimensions). If you like, maybe I can take a pass through and make it work w/ arrays similar to the last PR. I don't think the syntax would be any different, and it'll likely be an order of magnitude faster to use arrays.