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.

Sign up for GitHub

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

Jump to bottom

fixes-336: Extend the Ticker interface to allow passing a format function #362

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

fixes-336: Extend the Ticker interface to allow passing a format function #362

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a63a4c6
fixes-336: Extend the Ticker interface to allow passing a format func…
DavidGamba May 25, 2017
7e95849
fixes-336: Extend the Ticker interface to allow passing a format func…
DavidGamba Jun 18, 2017
068ee8d
fixes-336: Extend the Axis struct to have a Tick.Format field
DavidGamba Sep 5, 2017
9249345
fixes-336: Fix typo
DavidGamba Sep 5, 2017
6430bad
fixes-336: Expose DefaultTickFormat function
Sep 9, 2017
f4e7673
fixes-336: Update axis in-code documentation
DavidGamba Sep 16, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 28 additions & 16 deletions axis.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,10 @@ const displayPrecision = 4

// Ticker creates Ticks in a specified range
type Ticker interface {
// Ticks returns Ticks in a specified range
Ticks(min, max float64) []Tick
// Ticks returns Ticks in a specified range and formatted according to the
// given format function.
// When no format is provided (nil) a sane default is used.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// When format is nil DefaultTickFormat is used.

Sanity is subjective. We can claim this in talks, blogs and wiki articles, but I don't think it's appropriate in documentation.

Ticks(min, max float64, format func(v float64, prec int) string) []Tick
}

// Normalizer rescales values from the data coordinate system to the
Expand Down Expand Up @@ -74,6 +76,10 @@ type Axis struct {
// returned by the Marker function that are not in
// range of the axis are not drawn.
Marker Ticker

// Format function used to format the Axis Ticks.
// When no format is provided a sane default is used.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// When format is nil DefaultTickFormat is used.

Format func(v float64, prec int) string
}

// Scale transforms a value given in the data coordinate system
Expand Down Expand Up @@ -200,7 +206,7 @@ func (a *horizontalAxis) size() (h vg.Length) {
h -= a.Label.Font.Extents().Descent
h += a.Label.Height(a.Label.Text)
}
if marks := a.Tick.Marker.Ticks(a.Min, a.Max); len(marks) > 0 {
if marks := a.Tick.Marker.Ticks(a.Min, a.Max, a.Tick.Format); len(marks) > 0 {
if a.drawTicks() {
h += a.Tick.Length
}
Expand All @@ -220,7 +226,7 @@ func (a *horizontalAxis) draw(c draw.Canvas) {
y += a.Label.Height(a.Label.Text)
}

marks := a.Tick.Marker.Ticks(a.Min, a.Max)
marks := a.Tick.Marker.Ticks(a.Min, a.Max, a.Tick.Format)
ticklabelheight := tickLabelHeight(a.Tick.Label, marks)
for _, t := range marks {
x := c.X(a.Norm(t.Value))
Expand Down Expand Up @@ -254,7 +260,7 @@ func (a *horizontalAxis) draw(c draw.Canvas) {

// GlyphBoxes returns the GlyphBoxes for the tick labels.
func (a *horizontalAxis) GlyphBoxes(*Plot) (boxes []GlyphBox) {
for _, t := range a.Tick.Marker.Ticks(a.Min, a.Max) {
for _, t := range a.Tick.Marker.Ticks(a.Min, a.Max, a.Tick.Format) {
if t.IsMinor() {
continue
}
Expand All @@ -278,7 +284,7 @@ func (a *verticalAxis) size() (w vg.Length) {
w -= a.Label.Font.Extents().Descent
w += a.Label.Height(a.Label.Text)
}
if marks := a.Tick.Marker.Ticks(a.Min, a.Max); len(marks) > 0 {
if marks := a.Tick.Marker.Ticks(a.Min, a.Max, a.Tick.Format); len(marks) > 0 {
if lwidth := tickLabelWidth(a.Tick.Label, marks); lwidth > 0 {
w += lwidth
w += a.Label.Width(" ")
Expand All @@ -302,7 +308,7 @@ func (a *verticalAxis) draw(c draw.Canvas) {
c.FillText(sty, vg.Point{X: x, Y: c.Center().Y}, a.Label.Text)
x += -a.Label.Font.Extents().Descent
}
marks := a.Tick.Marker.Ticks(a.Min, a.Max)
marks := a.Tick.Marker.Ticks(a.Min, a.Max, a.Tick.Format)
if w := tickLabelWidth(a.Tick.Label, marks); len(marks) > 0 && w > 0 {
x += w
}
Expand Down Expand Up @@ -335,7 +341,7 @@ func (a *verticalAxis) draw(c draw.Canvas) {

// GlyphBoxes returns the GlyphBoxes for the tick labels
func (a *verticalAxis) GlyphBoxes(*Plot) (boxes []GlyphBox) {
for _, t := range a.Tick.Marker.Ticks(a.Min, a.Max) {
for _, t := range a.Tick.Marker.Ticks(a.Min, a.Max, a.Tick.Format) {
if t.IsMinor() {
continue
}
Expand All @@ -355,11 +361,14 @@ type DefaultTicks struct{}
var _ Ticker = DefaultTicks{}

// Ticks returns Ticks in a specified range
func (DefaultTicks) Ticks(min, max float64) (ticks []Tick) {
func (DefaultTicks) Ticks(min, max float64, format func(v float64, prec int) string) (ticks []Tick) {
const SuggestedTicks = 3
if max < min {
panic("illegal range")
}
if format == nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of sprinkling DefaultTicks and LogTicks with these if format == nil checks, perhaps we could instead put this default inside makeAxis ?
Like we do for Axis.Tick.Marker ?

and expose formatFloatTick as DefaultTickFormatter ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean:

  1. Creating an interface:
type TickFormatter interface {
    Format func(v float64, prec int) string
}

And having Axis.Format be of type TickFormatter and have a DefaultTickFormat struct implementing the interface and assigning that in makeAxis?

or

  1. Simply create a DefaultTickFormat function that is exposed and gets assigned to Axis.Format (of type func(v float64, prec int) string) in makeAxis?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(apologies for not being very clear)

the latter. this makes this new field (and its handling in the code) much more inline with the others.

format = formatFloatTick
}
tens := math.Pow10(int(math.Floor(math.Log10(max - min))))
n := (max - min) / tens
for n < SuggestedTicks {
Expand All @@ -379,7 +388,7 @@ func (DefaultTicks) Ticks(min, max float64) (ticks []Tick) {
prec := precisionOf(majorDelta)
for val <= max {
if val >= min && val <= max {
ticks = append(ticks, Tick{Value: val, Label: formatFloatTick(val, prec)})
ticks = append(ticks, Tick{Value: val, Label: format(val, prec)})
}
if math.Nextafter(val, val+majorDelta) == val {
break
Expand Down Expand Up @@ -421,24 +430,27 @@ type LogTicks struct{}
var _ Ticker = LogTicks{}

// Ticks returns Ticks in a specified range
func (LogTicks) Ticks(min, max float64) []Tick {
func (LogTicks) Ticks(min, max float64, format func(v float64, prec int) string) []Tick {
var ticks []Tick
val := math.Pow10(int(math.Floor(math.Log10(min))))
if min <= 0 {
panic("Values must be greater than 0 for a log scale.")
}
if format == nil {
format = formatFloatTick
}
prec := precisionOf(max)
for val < max*10 {
for i := 1; i < 10; i++ {
tick := Tick{Value: val * float64(i)}
if i == 1 {
tick.Label = formatFloatTick(val*float64(i), prec)
tick.Label = format(val*float64(i), prec)
}
ticks = append(ticks, tick)
}
val *= 10
}
tick := Tick{Value: val, Label: formatFloatTick(val, prec)}
tick := Tick{Value: val, Label: format(val, prec)}
ticks = append(ticks, tick)
return ticks
}
Expand All @@ -450,7 +462,7 @@ type ConstantTicks []Tick
var _ Ticker = ConstantTicks{}

// Ticks returns Ticks in a specified range
func (ts ConstantTicks) Ticks(float64, float64) []Tick {
func (ts ConstantTicks) Ticks(float64, float64, func(v float64, prec int) string) []Tick {
return ts
}

Expand Down Expand Up @@ -482,7 +494,7 @@ type TimeTicks struct {
var _ Ticker = TimeTicks{}

// Ticks implements plot.Ticker.
func (t TimeTicks) Ticks(min, max float64) []Tick {
func (t TimeTicks) Ticks(min, max float64, format func(v float64, prec int) string) []Tick {
if t.Ticker == nil {
t.Ticker = DefaultTicks{}
}
Expand All @@ -493,7 +505,7 @@ func (t TimeTicks) Ticks(min, max float64) []Tick {
t.Time = UTCUnixTime
}

ticks := t.Ticker.Ticks(min, max)
ticks := t.Ticker.Ticks(min, max, format)
for i := range ticks {
tick := &ticks[i]
if tick.Label == "" {
Expand Down
12 changes: 11 additions & 1 deletion axis_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,15 @@ package plot
import (
"math"
"reflect"
"strconv"
"testing"
)

func TestAxisSmallTick(t *testing.T) {
d := DefaultTicks{}
for _, test := range []struct {
min, max float64
format func(v float64, prec int) string
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should be a test where format is nil.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is where my lack of knowledge of the codebase comes in. Initially I added this check:

       if format == nil {
               format = formatFloatTick
       }

In DefaultTicks.Ticks and LogTicks.Ticks. However, @sbinet told me that the check wasn't required if I applied the DefaultTickFormat in makeAxis ( #362 (comment) ).
Right now, if you call DefaultTicks.Ticks directly without a format you get a panic in the test. I am assuming this will never happen but I don't have enough insight into it.

@sbinet and @kortschak please let me know what course to take on this one.

want []string
}{
{
Expand Down Expand Up @@ -41,8 +43,16 @@ func TestAxisSmallTick(t *testing.T) {
max: 0.00015,
want: []string{"0.0001", "0.00011", "0.00012", "0.00013", "0.00014"},
},
{
min: 0.0001,
max: 0.0005,
format: func(v float64, prec int) string {
return strconv.FormatFloat(v, 'e', 1, 64)
},
want: []string{"1.0e-04", "2.0e-04", "3.0e-04", "4.0e-04", "5.0e-04"},
},
} {
ticks := d.Ticks(test.min, test.max)
ticks := d.Ticks(test.min, test.max, test.format)
got := labelsOf(ticks)
if !reflect.DeepEqual(got, test.want) {
t.Errorf("tick labels mismatch:\ngot: %q\nwant:%q", got, test.want)
Expand Down
4 changes: 2 additions & 2 deletions gob/gob_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,8 @@ func randomPoints(n int, rnd *rand.Rand) plotter.XYs {
// into the labels for the major tick marks.
type commaTicks struct{}

func (commaTicks) Ticks(min, max float64) []plot.Tick {
tks := plot.DefaultTicks{}.Ticks(min, max)
func (commaTicks) Ticks(min, max float64, format func(v float64, prec int) string) []plot.Tick {
tks := plot.DefaultTicks{}.Ticks(min, max, format)
for i, t := range tks {
if t.Label == "" { // Skip minor ticks, they are fine.
continue
Expand Down
4 changes: 2 additions & 2 deletions plotter/grid.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func (g *Grid) Plot(c draw.Canvas, plt *plot.Plot) {
if g.Vertical.Color == nil {
goto horiz
}
for _, tk := range plt.X.Tick.Marker.Ticks(plt.X.Min, plt.X.Max) {
for _, tk := range plt.X.Tick.Marker.Ticks(plt.X.Min, plt.X.Max, plt.X.Tick.Format) {
if tk.IsMinor() {
continue
}
Expand All @@ -68,7 +68,7 @@ horiz:
if g.Horizontal.Color == nil {
return
}
for _, tk := range plt.Y.Tick.Marker.Ticks(plt.Y.Min, plt.Y.Max) {
for _, tk := range plt.Y.Tick.Marker.Ticks(plt.Y.Min, plt.Y.Max, plt.Y.Tick.Format) {
if tk.IsMinor() {
continue
}
Expand Down