Add map_values to Gradient #43
Conversation
arcadia_pycolor/gradient.py
Outdated
| return [mcolors.to_hex(cmap(0.5))] * len(values) | ||
|
|
||
| normalized_values = [(value - min_value) / (max_value - min_value) for value in values] | ||
| clamped_values = [max(0.0, min(1.0, value)) for value in normalized_values] |
There was a problem hiding this comment.
I wonder if we should warn the user when values outside of the min/max range are provided, rather than clamping those values silently here.
There was a problem hiding this comment.
I can only speak to my personal usage with colormaps, but if I'm touching the min and max values, it's usually because I'm interested in the middle of the color range, but far-reaching extrema values are decreasing contrast.
Like if your data is spread like this (each x a datapoint, each | the min/max value):
x x x x x x x x x x x x x x
| |
red yellow greenThe bulk of the data is washed out. In comparison, by setting min/max inside the value range:
x x x x x x x x x x x x x x
| |
red red yellow green greenYou get more contrast in the region you care about.
All this is to say that I personally would not want a warning that I'm doing this, because it would be very intentional.
There was a problem hiding this comment.
I understand this use case a bit better, thanks! I do wonder if it would be good to use the matplotlib set_extremes functionality when windowing the data in this way, so that colors over and under the gradient are distinctly marked as out of range. Probably not relevant to your structure viewer use case, but in a heatmap or other context, it would be important to indicate so as to avoid misleading readers.
There was a problem hiding this comment.
Yeah good idea. By name, I thought that set_extremes would allow us to avoid normalizing the data altogether, by setting the extrema for the value range. But looking into the implementation, I now see it allows you to control the colors of those outside 0 to 1.
This could be useful if we wanted more fancy features and could be accomplished by propagating those args to the map_values call signature.
Either way, it's noteworthy that the default behavior for over and under is to clamp the color to the lowest/highest color. Here's a black to white cmap:
(Pdb) mcolors.to_hex(cmap(-1.0))
'#000000'
(Pdb) mcolors.to_hex(cmap(2.0))
'#ffffff'
That's what we we were manually doing. As a step towards implementing over/under/bad, the clamping op has been removed (0c1c8af).
There was a problem hiding this comment.
Looks good to me! Thanks for addressing those comments.
Before merging, I do wonder if we should get a sense of how often we want to cut new releases on GitHub and distributions to PyPI. @keithchev any thoughts? I imagine it would make Evan's life a little easier if this is included in the PyPI-distributed version, but we should be disciplined about what amount of change necessitates a minor or patch version bump.
Yeah, if we end up with like version |
|
Sounds good! Feel free to merge @ekiefl. @keithchev would you mind distributing to PyPI? |
|
Merging now. Made a small change described here: #43 (comment) |
Add map_values method to Gradient
Proposed solution to #41
Something that gave me pause: on one hand I want to "buy in" to the existing data structures and make the return type
list[HexCode], but on the other hand I feel it's more practical to return the hex strings since I suspect any callers of this function are typically using it as a final step in their process with the library. I'm curious what you think @mezarque.