Skip to content
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

plot/palette: clean up API #352

Open
kortschak opened this issue Apr 30, 2017 · 4 comments
Open

plot/palette: clean up API #352

kortschak opened this issue Apr 30, 2017 · 4 comments
Assignees

Comments

@kortschak
Copy link
Member

kortschak commented Apr 30, 2017

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 kortschak self-assigned this Apr 30, 2017
@ctessum
Copy link
Contributor

ctessum commented Apr 30, 2017

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
Copy link
Contributor

ctessum commented Apr 30, 2017

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.

@kortschak
Copy link
Member Author

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

@ctessum
Copy link
Contributor

ctessum commented May 1, 2017

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants