plot: extending the Ticker interface

[hneemann]

Have you thought about extending the Ticker interface so that the font size and graphic width can be accessed? It seems a bit strange to me to create markers if you don't know the font size and how much space is available.

In my experience, you often want to have as many tick marks as possible as long as you can still read the labels, which means that they must not overlap.
To create such tick marks, however, you need to have access to the font size and the graphic size.

I have implemented this as an example to meet my requirements. Maybe this or a similar extension should be integrated.

[sbinet]

that kind of makes sense to me.

I would perhaps use this interface instead, though:

type Ticker interface {
	// Ticks returns Ticks in the specified range and
	// according to the given font definition.
	Ticks(min, max float64, font *vg.Font) []Tick
}

[sbinet]

do you have example plots for the rendering of your densed ticks?

[hneemann]

But you also need the axis width to calculate the marks. And if you only provide the font, you also need to know whether the axis is horizontal or vertical.

denseTickerExample.zip

[kortschak]

I think this tries to do to much. The point of the interface is that it is simple.

[hneemann]

@kortschak
Perhaps you're right.
But to me, implementing this interface feels like answering the question "How many letters fit in this line?" and neither knowing the length of the line nor the width of a letter.

[kortschak]

It doesn't do that though. You are attempting to use the information to decide how far apart text should be on an axis, where the axis is defined by a plot.Axis, which contains a draw.TextStyle which allows rotation. The minimum information required would be for the ticker to hold the target axis (which would include the orientation of the axis by virtue of its type). This would mean that we'd need to export horizontalAxis and verticalAxis. To make effective use of the information we then need to somehow formalise what it means for a spacing to look good, but this is an aesthetic decision. Sometimes, you just need a human.

[hneemann]

Sometimes, you just need a human.

This is not an option if you don't have an interactive system to design the plot, but the plot is created in a batch process. In this case aesthetics is not that important. In this case I want to display as many markers as possible without overlapping to make the chart as readable as possible. And that's very simple, you don't need a human being to do that. But you need a bit more information than is available now.

But you can also specify the axis and its orientation when generating the ticker. This would work and you wouldn't have to change anything, But it undermines the ticker's ability to be independent of the axis and its orientation.

[kortschak]

If that's the case you can define a func(min, max float64, horiz bool, plot.Axis) plot.Ticker that does the calculation for you.

[hneemann]

I've tried that, but it turns out to be quite messy. The current design makes it difficult to access the required dimensions.
I rather stay with the fork that I have created. That seems more reasonable.

Anyway, thanks for your time.

[kortschak]

I'm a bit confused about why. The plot.Axis holds the text style and rotation which means you know everything about the extents of the font. These are things that would need to otherwise determine to satisfy your StringSizer type implementations. Without the implementation being used it's difficult to see how it works.

[sbinet]

I came up with this little program that does what you want.
the one thing that may not be completely satisfying is the way to derive a draw.Canvas.
on the plus side: no need to fork (which is always a nice thing: I did that at some point for my go-hep/hplot thing and it was always a bit messy)

package main

import (
	"image/color"
	"log"
	"math/rand"

	"gonum.org/v1/plot"
	"gonum.org/v1/plot/plotter"
	"gonum.org/v1/plot/vg"
	"gonum.org/v1/plot/vg/draw"
	"gonum.org/v1/plot/vg/vgimg"
)

func main() {
	p, err := plot.New()
	if err != nil {
		log.Fatalf("%+v", err)
	}

	p.Title.Text = "random data"
	p.X.Label.Text = "x"
	p.Y.Label.Text = "y"

	var (
		w = 10 * vg.Centimeter
		h = 10 * vg.Centimeter
	)

	p.X.Tick.Marker = newXTicker(p.X.Tick.Marker, p, w, h)
	p.Y.Tick.Marker = newYTicker(p.Y.Tick.Marker, p, w, h)

	fill(p)

	_ = p.Save(w, h, "foo.png")
}

type MyTicker struct {
	ticker plot.Ticker

	width func() vg.Length
	size  func(s string) vg.Length
}

func newTicker(ticker plot.Ticker, width func() vg.Length, size func(s string) vg.Length) *MyTicker {
	if ticker == nil {
		ticker = new(plot.DefaultTicks)
	}
	return &MyTicker{
		ticker: ticker,
		width:  width,
		size:   size,
	}
}

func newXTicker(ticker plot.Ticker, p *plot.Plot, w, h vg.Length) *MyTicker {
	c := draw.New(vgimg.New(w, h))
	width := func() vg.Length {
		max := p.X.Norm(p.X.Max)
		min := p.X.Norm(p.X.Min)
		return c.X(max) - c.X(min)
	}

	size := p.X.Tick.Label.Font.Width

	return newTicker(ticker, width, size)
}

func newYTicker(ticker plot.Ticker, p *plot.Plot, w, h vg.Length) *MyTicker {
	c := draw.New(vgimg.New(w, h))
	width := func() vg.Length {
		max := p.Y.Norm(p.Y.Max)
		min := p.Y.Norm(p.Y.Min)
		return c.Y(max) - c.Y(min)
	}

	size := func(s string) vg.Length {
		return p.Y.Tick.Label.Font.Size
	}

	return newTicker(ticker, width, size)
}

func (mt MyTicker) Ticks(min, max float64) []plot.Tick {
	log.Printf("size= %v", mt.size("hello"))
	log.Printf("width= %v", mt.width())
	log.Printf("min=%v", min)
	log.Printf("max=%v", max)

	return mt.ticker.Ticks(min, max)
}

func fill(p *plot.Plot) {
	rnd := rand.New(rand.NewSource(1))

	// randomPoints returns some random x, y points
	// with some interesting kind of trend.
	randomPoints := func(n int) plotter.XYs {
		pts := make(plotter.XYs, n)
		for i := range pts {
			if i == 0 {
				pts[i].X = rnd.Float64()
			} else {
				pts[i].X = pts[i-1].X + rnd.Float64()
			}
			pts[i].Y = pts[i].X + 10*rnd.Float64()
		}
		return pts
	}

	s, err := plotter.NewScatter(randomPoints(15))
	if err != nil {
		log.Panic(err)
	}
	s.GlyphStyle.Color = color.RGBA{R: 255, B: 128, A: 255}
	s.GlyphStyle.Radius = vg.Points(3)

	p.Add(s, plotter.NewGrid())

}

[hneemann]

Yep, that was also my problem: I could not access the canvas.
But passing the canvas to the Ticks method, or the appropriate func(x float64) vg.Length function of the canvas feels weird, too.

[hneemann]

What about something like this:

type RenderContext struct {
	Axis        Axis
	Orientation bool
	Canvas      Canvas
}


type Ticker interface {
	// Ticks returns Ticks in the specified range and
	// according to the given font definition.
	Ticks(min, max float64, rct RenderContext) []Tick
}

It adds not to much clutter to the Ticker interface and it helps in the Ticker implementation because it's very easy to pass all required information to some kind of factory method.

[kortschak]

The issue I have with this is the impact that it would have on importing packages. While we are free to break downstream, we try to avoid it if at all possible. The benefits to be gained need to be significant if we are going to break the users of plot. At the moment, the issue that you have can be worked around without a change in API, and it's not entirely clear how the additional API would interact with the existing tickers that we provide (Lin Hanrahan and Talbot for example is agnostic to the information that the addition would provide); this suggests that the parameters for the axis and canvas should be fields in the implementations' structs.

Fundamentally, the API change breaks a design decision that was made when the plot package was originally written, that the data side and the rendering side of plotting are separate entities. If you want to move forward with this, I'd like to see a more detailed proposal that explains how the system would work.

[hneemann]

Fundamentally, the API change breaks a design decision that was made when the plot package was originally written, that the data side and the rendering side of plotting are separate entities. If you want to move forward with this, I'd like to see a more detailed proposal that explains how the system would work.

These are my thoughts on that:

1. If you want to create tick marks for an axis, it is important to know how wide the axis is. Imagine you are drawing a plot by hand on paper, and you want to show the values from 0 to 100. To find the tick marks it is important to know if the plot should be 30cm, 20cm or only 5cm wide.
2. Currently this information is difficult to obtain, as shown by @sbinet: Creating a canvas, extracting a function from it, then discarding the canvas and hoping that a new similar canvas will be created later on to draw the plot is at best a fragile hack.
3. This difficulty shows that something is missing in the current API.
4. This weakness should be addressed.

Would you agree with these conclusions? If I'm the only one who sees it that way, there's obviously no point for me to do anything about it. Which I would also be perfectly fine with.

[kortschak]

I've just had another look at the plot and axis code and ISTM that the canvas is not needed. plot.Axis already knows nearly everything that is needed; as I mentioned above, it knows about the font, but it also has the scaling of the axis graphics coordinate from the data value coordinates in the Scale field. What it doesn't know is the orientation of the axis, although the called is the horizontalAxis or verticalAxis type, so it is possible for the caller to hand over an appropriately adjusted draw.TextStyle from the Axis.Tick.Label field; the value of Rotation adjusted so that it is rotated -pi/2 before it's passed to the Ticks method.

This ultimately means that only the Axis field needs to be passed in (with the mutated Label field - or possibly with a bigger API change that makes Axis an interface type, which I'm not wildly keen on) and we have Ticks(min, max float64, axis plot.Axis) []plot.Tick. However, it also shows that you could do the same thing with the function signature I wrote above with not a huge amount of work by making a ConstantTicks using that information.

[sbinet]

at this rate, it could just be:

type Ticker interface {
    Ticks(axis Axis) []Tick
}

but there's still the issue of computing the width of the axis (for that, IIUC, one needs a draw.Canvas)

[hneemann]

To be honest, I don't get it!
As I understand it, the Normalizer stored in the Scale field only returns values between 0 and 1. The X and Y functions of the canvas are required to get the correct axis width.
But maybe I'm overlooking something.
Can you help me out and post a minimal ticker example that writes the length of the axis and the width of the letter "0" in the same unit of measurement to the console?

[kortschak]

Sorry, you're right (cold-muddled head). I think I'd be happy with

type Ticker interface {
    Ticks(Axis, vg.Length) []Tick
}

where the length is calculated by c.X(1)-c.X(0) for example for the horizontal case. Do you think that would work for you? I'm still concerned that 1. it breaks a lot of people and 2. the length is not always usable (it's essentially only a hint for some cases).

[sbinet]

what could be done to mitigate the breakage is to expose a new, different, interface that would be leveraged optionally (like is done in, e.g., io.Copy)

e.g.:

type TickerFrom interface {
    TicksFrom(a Axis, w vg.Length) []Tick
}

// ...

func (a horizontalAxis) size() (h vg.Length) {
	if a.Label.Text != "" { // We assume that the label isn't rotated.
		h -= a.Label.Font.Extents().Descent
		h += a.Label.Height(a.Label.Text)
	}

	var marks []Ticks
	switch ticker := a.Tick.Marker.(type) {
	case TickerFrom:
		marks = ticker.TicksFrom(a, width)
	default:
		marks = a.Tick.Marker.Ticks(a.Min, a.Max)
	}
	if len(marks) > 0 {
		if a.drawTicks() {
			h += a.Tick.Length
		}
		h += tickLabelHeight(a.Tick.Label, marks)
	}
	h += a.Width / 2
	h += a.Padding

	return h
}

and document.

[hneemann]

Or you simply add a new property to the Axis struct. Say AxisLength vg.Length
This would break no existing code. But you have to update this field every time ticker.Ticks is called which adds a hidden implicit dependency which is a bad thing to do.

What also is possible is this solution:

type Ticker interface {
	Ticks(min, max float64) []Tick
}

type TickerFrom interface {
	Ticker
	TicksFrom(min, max float64, rct *RenderContext) []Tick
}

type impl struct {
}

func (i impl) Ticks(min, max float64) []Tick {
	return i.TicksFrom(min, max, nil)
}

func (impl) TicksFrom(min, max float64, rct *RenderContext) []Tick {
	fmt.Println("go!")
	return nil
}

func createTicks(min, max float64, ticker Ticker) []Tick {
	if contextTicker, ok := ticker.(TickerFrom); ok {
		return contextTicker.TicksFrom(min, max, &RenderContext{})
	} else {
		return ticker.Ticks(min, max)
	}
}

This extention also breaks no existing code, and the RenderContext gives you the freedom of modifications in the future, by adding fields to RenderContext. But every one who implements the new TickerFrom interface has to deal with the old Ticks method. Also not the best thing to do.

[kortschak]

I don't really like the need to double up the methods. An alternative is to have a conditional setter that hands the axis and length to the ticker for it to work from; SetAxis(Axis, vg.Length).

I'd rather not add a new type.

[hneemann]

So you are thinking about something like this?

type impl struct {
}

func (i impl) Ticks(min, max float64) []plot.Tick {
	return nil
}

func (i impl) SetAxis(axis plot.Axis, len vg.Length, orientation bool) {
}

func createTicks(min, max float64, ticker Ticker, axis plot.Axis, len vg.Length, orientation bool) []plot.Tick {
	type setter interface {
		SetAxis(plot.Axis, vg.Length, bool)
	}

	if hasSetter, ok := ticker.(setter); ok {
		hasSetter.SetAxis(axis, len, orientation)
	}

	return ticker.Ticks(min, max)
}

[kortschak]

Something like that, yes. Orientation is not a bool though, maybe horizontal bool. Though I still think a boolean for an direction is yucky.

[hneemann]

Though I still think a boolean for an direction is yucky.

I agree!

[sbinet]

Though I still think a boolean for an direction is yucky.

I agree!

that's fixed (w/ #583 ) :)

[hneemann]

Up to now I like this the most:

type Ticker interface {
	Ticks(min, max float64) []plot.Tick
}

type SizedTicker interface {
	Ticker
	SetAxis(axis *plot.Axis, width vg.Length, orientation Orientation)
}

type impl struct {
}

var _ SizedTicker = impl{}

func (i impl) Ticks(min, max float64) []plot.Tick {
	return nil
}

func (i impl) SetAxis(axis *plot.Axis, len vg.Length, orientation Orientation) {
	fmt.Println("use sizes")
}

func createTicks(min, max float64, ticker Ticker, axis *plot.Axis, canvas draw.Canvas) []plot.Tick {
	if needsAxis, ok := ticker.(SizedTicker); ok {
		width, orientation := axis.getWidthAndOrientation(canvas)
		needsAxis.SetAxis(axis, width, orientation)
	}

	return ticker.Ticks(min, max)
}

This will not break any existing code and looks clean to me.

A completely invisible method adds some kind of magic to the API.
And things that work magically will also break magically at some time.

[hneemann]

Here you can find a prototype implementation of this enhancement. The repo includes a package denseTest which contains a example usage of this new capabilities. It shows how the markers adapt to the size of the output.

BTW: There is a rounding issue in plot.rightMost and plot.leftMost which i also have fixed. While in canvas.isLeft, canvas.isRight, canvas.isBelow and canvas.isAbove a rounding error is tolerated, this is not the case in plot.rightMost and plot.leftMost.

[kortschak]

I hope to get to look at this reasonably soon.