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 1 commit
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
36 changes: 21 additions & 15 deletions axis.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ const displayPrecision = 4
// Ticker creates Ticks in a specified range
type Ticker interface {
// Ticks returns Ticks in a specified range
Copy link
Member

Choose a reason for hiding this comment

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

if the interface is modified, the new argument should also be documented.

Ticks(min, max float64) []Tick
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 @@ -200,7 +200,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, nil); len(marks) > 0 {
Copy link
Member

Choose a reason for hiding this comment

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

ok. so how would I use my own format here?

shouldn't Axis grow another field that could be passed there?
something like:

type Axis struct {
   // ...
   Tick struct {
      // ...
      // Marker returns the tick marks.  Any tick marks
      // returned by the Marker function that are not in
      // range of the axis are not drawn.
      Marker Ticker

      // <doc for Format>
      Format func(v float64, prec int) string
   }
}

and then:

if marks := a.Tick.Marker.Ticks(a.Min, a.Max, a.Tick.Format); len(marks) > 0 {

Copy link
Author

Choose a reason for hiding this comment

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

I'll give it a shot 😃 Thanks for the feedback!

if a.drawTicks() {
h += a.Tick.Length
}
Expand All @@ -220,7 +220,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, nil)
ticklabelheight := tickLabelHeight(a.Tick.Label, marks)
for _, t := range marks {
x := c.X(a.Norm(t.Value))
Expand Down Expand Up @@ -254,7 +254,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, nil) {
if t.IsMinor() {
continue
}
Expand All @@ -278,7 +278,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, nil); len(marks) > 0 {
if lwidth := tickLabelWidth(a.Tick.Label, marks); lwidth > 0 {
w += lwidth
w += a.Label.Width(" ")
Expand All @@ -302,7 +302,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, nil)
if w := tickLabelWidth(a.Tick.Label, marks); len(marks) > 0 && w > 0 {
x += w
}
Expand Down Expand Up @@ -335,7 +335,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, nil) {
if t.IsMinor() {
continue
}
Expand All @@ -355,11 +355,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 +382,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 +424,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 +456,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 +488,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 +499,7 @@ func (t TimeTicks) Ticks(min, max float64) []Tick {
t.Time = UTCUnixTime
}

ticks := t.Ticker.Ticks(min, max)
ticks := t.Ticker.Ticks(min, max, nil)
for i := range ticks {
tick := &ticks[i]
if tick.Label == "" {
Expand Down
2 changes: 1 addition & 1 deletion axis_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func TestAxisSmallTick(t *testing.T) {
want: []string{"0.0001", "0.00011", "0.00012", "0.00013", "0.00014"},
},
} {
ticks := d.Ticks(test.min, test.max)
ticks := d.Ticks(test.min, test.max, nil)
Copy link
Member

Choose a reason for hiding this comment

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

you should also add a test with the new format argument

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, nil)
Copy link
Member

Choose a reason for hiding this comment

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

you should probably weave the format arg through the call of DefaultTicks, instead of dropping it on the floor

Copy link
Member

Choose a reason for hiding this comment

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

actually, I don't understand why weaving format through all the calls to .Ticks(...) isn't done.

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, nil) {
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, nil) {
if tk.IsMinor() {
continue
}
Expand Down