plot icon indicating copy to clipboard operation
plot copied to clipboard

plot/palette: clean up API

Open kortschak opened this issue 7 years ago • 4 comments

I am troubled by the size of the ColorMap interface. It can be reduced in size to

// A ColorMap maps scalar values to colors.
type ColorMap interface {
	// At returns the color associated with the given value.
	// If the value is not between Max() and Min(), an error is returned.
	At(float64) (color.Color, error)

	// Min returns the current minimum value of the ColorMap.
	Min() float64

	// Max returns the current maximum value of the ColorMap.
	Max() float64

	// Palette returns a new Palette with the specified number of
	// colors from the ColorMap.
	Palette(colors int) Palette
}

and

// DivergingColorMap maps scalar values to colors that diverge from a central value.
type DivergingColorMap interface {
	ColorMap

	// ConvergePoint returns the value where the diverging colors meet.
	ConvergePoint() float64
}

The Set methods are required because the concrete types are not exported. This can be fixed with minor name juggling on the field names of the concrete types. For example:

// Luminance is a color palette that interpolates between control colors in a way that
// ensures a linear relationship between the luminance of a color and the value it represents.
// NewLuminance must be used to create a usable Luminance.
type Luminance struct {
	// colors are the control colors to be interpolated among.
	// The colors must be monotonically increasing in luminance.
	colors []cieLAB

	// scalars are the scalar control points associated with
	// each item in colors (above). They are monotonically
	// increasing values between zero and one that correspond
	// to the luminance of a given control color in relation
	// to the minimum and maximum luminance among all control
	// colors.
	scalars []float64

	// Alpha represents the opacity of the returned colors in
	// the range (0,1). NewLuminance sets Alpha to 1.
	Alpha float64

	// Range specifies the minimum and maximum values of the
	// range of scalars that can be mapped to colors using this
	// ColorMap.
	Range struct { Min, Max float64 }
}

kortschak avatar Apr 30 '17 00:04 kortschak

Makes sense to me.

On this topic, additionally, I'm somewhat confused regarding what the purpose of the palette.Palette interface is. It only has one method, and the method doesn't take any arguments, so anything that is being used as a palette has to know ahead of time what colors it is going to return. Given that, would it make more sense to do type Palette []color.Color or just delete the Palette type altogether and use []color.Color instead?

ctessum avatar Apr 30 '17 19:04 ctessum

I suppose one reason is that without a Palette interface we couldn't have a DivergingPalette. That may be enough to justify it, but it still feels to me like a lot of complexity just for that.

ctessum avatar Apr 30 '17 19:04 ctessum

palette.Palette is justified by the same argument as fmt.Stringer.

kortschak avatar Apr 30 '17 21:04 kortschak

The functions in fmt are set up so they can take either a string or a fmt.Stringer (or anything else). So the existence of fmt.Stringer doesn't prevent people from using plain strings in the functions. In this case, though, we have to choose between accepting []color.Color or palette.Palette; there doesn't seem to be a good way to accept both. I agree, though, that on balance palette.Palette is probably the better choice.

ctessum avatar May 01 '17 00:05 ctessum