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

DavidGamba · 2017-05-25T06:04:06Z

Fixes #336 This allows for easy wrappers around the plot.DefaultTicks struct with
custom format functions:

    type myCustomFormatTicks struct{}

    var _ plot.Ticker = myCustomFormatTicks{}

    func (myCustomFormatTicks) Ticks(min, max float64, format func(v float64, prec int) string) (ticks []plot.Tick) {
      return plot.DefaultTicks{}.Ticks(min, max, myFormatFloatTick)
    }

    func myFormatFloatTick(v float64, prec int) string {
           return strconv.FormatFloat(floats.Round(v, prec), 'g', 8, 64)
    }

    p.Y.Tick.Marker = myCustomFormatTicks{}

…tion This allows for easy wrappers around the plot.DefaultTicks struct with custom format functions: type myCustomFormatTicks struct{} var _ plot.Ticker = myCustomFormatTicks{} func (myCustomFormatTicks) Ticks(min, max float64, format func(v float64, prec int) string) (ticks []plot.Tick) { return plot.DefaultTicks{}.Ticks(min, max, myFormatFloatTick) } func myFormatFloatTick(v float64, prec int) string { return strconv.FormatFloat(floats.Round(v, prec), 'g', 8, 64) } p.Y.Tick.Marker = myCustomFormatTicks{}

sbinet · 2017-05-26T08:09:01Z

gob/gob_test.go

-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)


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

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

sbinet · 2017-05-26T08:14:10Z

axis.go

@@ -21,7 +21,7 @@ const displayPrecision = 4
 // Ticker creates Ticks in a specified range
 type Ticker interface {
 	// Ticks returns Ticks in a specified range


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

sbinet · 2017-05-26T08:16:28Z

axis_test.go

@@ -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)


you should also add a test with the new format argument

…tion * Document the extra argument in the Ticker interface. * Weave the format arg through the call to DefaultTicks when applicable. * Add an extra test with the additional argument.

DavidGamba · 2017-06-18T02:59:08Z

@sbinet please take another look.

sbinet · 2017-06-20T16:43:49Z

axis.go

@@ -200,7 +202,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 {


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 {

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

sbinet · 2017-06-20T17:09:15Z

@kortschak @eaburns any feedback about this change?

Allows to define a custom Tick format function.

DavidGamba · 2017-09-05T04:08:40Z

@sbinet please take another look. I apologize for the delay...
With this, to provide a format to the default ticks, all I have to do is:

...
	p, err := plot.New()
	if err != nil {
		return err
	}
	p.X.Tick.Format = myAxisTickFormat
	p.Y.Tick.Format = myAxisTickFormat
...

func myAxisTickFormat(v float64, prec int) string {
	return strconv.FormatFloat(floats.Round(v, prec), 'f', 0, 64)
}

sbinet · 2017-09-05T07:39:13Z

axis.go

 	const SuggestedTicks = 3
 	if max < min {
 		panic("illegal range")
 	}
+	if format == nil {


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 ?

Do you mean:

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

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

(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.

And set it by default on axis.makeAxis

DavidGamba · 2017-09-09T04:18:12Z

@sbinet PTAL

sbinet

LGTM.

@kortschak and/or @eaburns should definitely comment, though :)

kortschak · 2017-09-11T00:11:46Z

axis.go

@@ -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.


// When format is nil DefaultTickFormat is used.

kortschak · 2017-09-11T00:12:15Z

axis.go

-	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.


// 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.

kortschak · 2017-09-11T00:14:28Z

axis_test.go

 	"testing"
 )

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


There should be a test where format is nil.

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.

This was referenced May 25, 2017

Add IntTicks #360

Closed

Tick display precision should be configurable #336

Open

sbinet reviewed May 26, 2017

View reviewed changes

fixes-336: Extend the Ticker interface to allow passing a format func…

7e95849

…tion * Document the extra argument in the Ticker interface. * Weave the format arg through the call to DefaultTicks when applicable. * Add an extra test with the additional argument.

sbinet reviewed Jun 20, 2017

View reviewed changes

fixes-336: Extend the Axis struct to have a Tick.Format field

068ee8d

Allows to define a custom Tick format function.

fixes-336: Fix typo

9249345

sbinet reviewed Sep 5, 2017

View reviewed changes

fixes-336: Expose DefaultTickFormat function

6430bad

And set it by default on axis.makeAxis

sbinet previously approved these changes Sep 10, 2017

View reviewed changes

kortschak requested changes Sep 11, 2017

View reviewed changes

fixes-336: Update axis in-code documentation

f4e7673

sbinet dismissed their stale review via f4e7673 August 3, 2021 07:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

DavidGamba commented May 25, 2017 •

edited

Loading

sbinet May 26, 2017

sbinet May 26, 2017

sbinet May 26, 2017

sbinet May 26, 2017

DavidGamba commented Jun 18, 2017

sbinet Jun 20, 2017

DavidGamba Jun 20, 2017

sbinet commented Jun 20, 2017

DavidGamba commented Sep 5, 2017

sbinet Sep 5, 2017

DavidGamba Sep 6, 2017

sbinet Sep 6, 2017

DavidGamba commented Sep 9, 2017

sbinet left a comment

kortschak Sep 11, 2017

kortschak Sep 11, 2017

kortschak Sep 11, 2017

DavidGamba Sep 16, 2017

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

Are you sure you want to change the base?

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

Conversation

DavidGamba commented May 25, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DavidGamba commented Jun 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sbinet commented Jun 20, 2017

DavidGamba commented Sep 5, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DavidGamba commented Sep 9, 2017

sbinet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DavidGamba commented May 25, 2017 •

edited

Loading