-
Notifications
You must be signed in to change notification settings - Fork 204
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
A color scale is tied to a single heat map, making it impossible to have multiple heatmaps with the same color scale #304
Comments
I don't believe this is true. If the same palette is passed to NewHeatmap or used in the struct declaration they will be using the same colour mapping, provided the min and max are conformed to the same range. This is trivial after all the Heatmaps have been created.
The legend colour map would just be one of the elements of h. |
That makes sense. I think, though, there still might be a rationale for separating the color map from the heat map, as having a standalone color map could enable functionality such as:
Food for thought. |
I could see that working, though I don't really understand how it help with some of the things you list, notable, bullet points 1-3. The last bullet point would find a fair amount of additional API to achieve it. One of the things that needs to be considered here is that plot does (intentionally from @eaburns AFAIUI and certainly from me) a minimal amount of things to get a plot done (in some cases it actually does too much in my view and starts to feel like a framework - debugging things can be difficult because of the amount of data tracing required). This is quite unlike other plotting environments (e.g. ggplot2) which hide most things and perform a lot of magic. These packages make simple/common things easy, but unknown/complicated things virtually impossible. I hope we don't end up in that space. |
The functionality in question here is assigning colors to data points. Currently, HeatMap is the only part of the package that does this, but there are a bunch of potential applications of this functionality (eg, 1, 2, 3). There is already at least one (@btracey) external plotter that does it independently. To me, the question is whether every plotter that needs to assign colors to data points should have that logic built into it independently, or whether it should be separated out and implemented once. The answer to that question probably depends whether having a general function/interface for assigning colors to data points is worth the added API surface (which I feel it does because I use that functionality a lot in different contexts, but others may disagree), but I don't think it would necessarily increase the level of "magic". An example API could be type ColorMap interface {
Color(float64) (color.Color, error) // the argument could also be an interface{} for more generality
Legend(vertical bool) (*plot.Plot, error)
}
type LinearColorMap struct {
// Palette is the color palette used to render
// the color map. Palette must not be nil or
// return a zero length []color.Color.
Palette palette.Palette
// Underflow and Overflow are colors used to fill
// color map elements outside the dynamic range
// defined by Min and Max.
Underflow color.Color
Overflow color.Color
// Min and Max define the dynamic range of the
// color map.
Min, Max float64
}
func (cm *LinearColorMap) Color(v float64) (color.Color, error) {
// Logic from heatmap.Plot
}
func (cm *LinearColorMap) Legend(vertical bool) (*plot.Plot, error) {
// Logic adapted from PR #290
} |
OK, I understand now. That looks reasonable. The entanglement that I don't like is how Min and Max are used by the plotter. One approach would be that a plotter will only expand the range of a ColorMap type held by it. This would give a natural result equivalent to my snippet above. I would not think an interface{} param for Color would be a good idea. Are we happy with the notion of a plot.Plot being a panel unit? It feels ungainly to me, but I guess that's something I could get over given time. |
Having Legend return a plot.Plot allows a lot of customizability with tick marks, titles, etc that would otherwise have to be duplicated, but it might be possible with some refactoring to have a ColorLegend type that includes the necessary parts of a plot.Plot without actually being a plot.Plot and with minimal code duplication. Not sure if that would be worth the work though. A solution to the entanglement could be to not have the plotters edit the color map at all, but instead require Min and Max to be set by the user or have a ColorMap.ExpandRange(...float64) method. The reason to have an interface{} param for Color() would be to allow an implementation of categorical color maps, which would assign different colors to categories like "apple farms", "orange farms", etc, and therefore would require a string input instead of a float. This might be a case where the added functionality does not warrant the added confusion, however. |
Mmmm. This is what I was thinking about in my comment above, but I subsequently forgot. Losing the automatic range determination would not be a good idea IMO since then all users would always need to determine ranges - from where and how? ExpandRange feels gross, but I guess would be OK, though not with a variadic signature. Maybe Include(float64).
These should be distinct type I think. |
It could be fixed by making the color-mapper separate from the heat map, and then either requiring the color-mapper to be an argument to NewHeatMap, or requiring GridXYZ to be a matrix of colors rather than a matrix of floats.
Making these separate would also be useful in other areas, such as allowing plots with points or lines that are colored according to some attribute of the data.
I think this is related to #302.
The text was updated successfully, but these errors were encountered: