Skip to content

Implemented an AutoUnixTimeTicks #313

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

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

Implemented an AutoUnixTimeTicks #313

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ce11165
Implemented an AutoUnixTimeTicks, which should probably be the default
abourget Sep 12, 2016
7832dac
Updated to take comments into account.
abourget Sep 29, 2016
d772503
Renamed AutoUnixTimeTicks to TimeTicks.
abourget Sep 29, 2016
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
153 changes: 153 additions & 0 deletions axis.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package plot
import (
"image/color"
"math"
"os"
"strconv"
"time"

Expand Down Expand Up @@ -497,6 +498,158 @@ func (utt UnixTimeTicks) Ticks(min, max float64) []Tick {
return ticks
}

// AutoUnixTimeTicks is suitable for axes representing time values.
// AutoUnixTimeTicks expects values in Unix time seconds. It will
// adjust the number of ticks according to the specified Width. If
// not specified, Width defaults to 4 inches.
type AutoUnixTimeTicks struct {
Copy link
Copy Markdown
Member

@kortschak kortschak Sep 12, 2016

Choose a reason for hiding this comment

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

I'm not convinced by the name.

Copy link
Copy Markdown
Author

@abourget abourget Sep 29, 2016

Choose a reason for hiding this comment

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

heh.. I know it isn't pretty :) UnixTimeTicks was already taken.. and Auto was used to distinguish the added feature..

Could we make it TimeTicks ? and document the fact that you need Unix timestamps, or provide a function to transform a []time.Time into whatever this thing needs ?

// Width is the width of the underlying graph, used to calculate
Copy link
Copy Markdown
Member

@kortschak kortschak Jun 28, 2017

Choose a reason for hiding this comment

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

s/graph/plot/

// the number of ticks that can fit properly with their time
// shown.
Width vg.Length
}

var _ Ticker = AutoUnixTimeTicks{}

// Inspired by https://github.com/d3/d3-scale/blob/master/src/time.js
var tickIntervals = []tickRule{
Copy link
Copy Markdown
Member

@kortschak kortschak Sep 12, 2016

Choose a reason for hiding this comment

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

tickRules

{time.Millisecond,
Copy link
Copy Markdown
Member

@kortschak kortschak Sep 12, 2016

Choose a reason for hiding this comment

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

Don't break these over two lines.

"15:04:05.000", "15:04:05", ".000", time.Millisecond},
{200 * time.Millisecond,
"15:04:05.000", "15:04:05", ".000", 200 * time.Millisecond},
{500 * time.Millisecond,
"15:04:05.000", "15:04:05", ".000", 500 * time.Millisecond},
{time.Second,
"15:04:05", "15:04", ":05", time.Second},
{2 * time.Second,
"15:04:05", "15:04", ":05", 2 * time.Second},
{5 * time.Second,
"15:04:05", "15:04", ":05", 5 * time.Second},
{15 * time.Second,
"Jan 02, 15:04", "Jan 02", "15:04", 15 * time.Second},
{30 * time.Second,
"15:04:05", "15:04", ":05", 30 * time.Second},
{time.Minute,
"15:04:05", "15:04", ":05", time.Minute},
{2 * time.Minute,
"Jan 02, 3:04pm", "Jan 02", "3:04pm", 2 * time.Minute},
{5 * time.Minute,
"Jan 02, 3:04pm", "Jan 02", "3:04pm", 5 * time.Minute},
{15 * time.Minute,
"Jan 02, 3:04pm", "Jan 02", "3:04pm", 15 * time.Minute},
{30 * time.Minute,
"Jan 02, 3:04pm", "Jan 02", "3:04pm", 30 * time.Minute},
{time.Hour,
"Jan 2, 3pm", "Jan 2", "3pm", time.Hour},
{3 * time.Hour,
"Jan 2, 3pm", "Jan 2", "3pm", 3 * time.Hour},
{6 * time.Hour,
"Jan 2, 3pm", "Jan 2", "3pm", 6 * time.Hour},
{12 * time.Hour,
"Jan 2", "Jan 2", "3pm", 12 * time.Hour},
{24 * time.Hour,
"Jan 2", "Jan", "2", 24 * time.Hour},
{48 * time.Hour,
"Jan 2", "Jan", "2", 48 * time.Hour},
{7 * 24 * time.Hour,
"Jan 2", "Jan", "2", 7 * 24 * time.Hour},
{aMonth, // 1 month
"Jan 2006", "2006", "Jan", aMonth},
{3 * aMonth, // 3 months
"Jan 2006", "2006", "Jan", 3 * aMonth},
{6 * aMonth, // 6 months
"Jan 2006", "2006", "Jan", 6 * aMonth},
{12 * aMonth, // 1 year
"2006", "", "2006", 12 * aMonth},
{2 * 12 * aMonth, // 2 years
"2006", "", "2006", 2 * 12 * aMonth},
{5 * 12 * aMonth, // 5 years
"2006", "", "2006", 5 * 12 * aMonth},
{10 * 12 * aMonth, // 10 years
"2006", "", "2006", 10 * 12 * aMonth},
}

var aMonth = 31 * 24 * time.Hour
Copy link
Copy Markdown
Member

@sbinet sbinet Sep 12, 2016

Choose a reason for hiding this comment

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

could probably be a const instead of a var ?

Copy link
Copy Markdown
Member

@kortschak kortschak Sep 12, 2016

Choose a reason for hiding this comment

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

Called month.


type tickRule struct {
Copy link
Copy Markdown
Member

@kortschak kortschak Sep 12, 2016

Choose a reason for hiding this comment

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

Doc comments here as complete grammatical sentences please.

Copy link
Copy Markdown
Member

@kortschak kortschak Jun 28, 2017

Choose a reason for hiding this comment

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

This type does not need to exist. You can define tickRules on the anonymous struct.

DurationPerInch time.Duration // use this rule for a maximum Duration per inch
Copy link
Copy Markdown
Member

@kortschak kortschak Sep 12, 2016

Choose a reason for hiding this comment

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

Please either an SI or typesetting unit. SI would be nice, but we have what we have. Maybe express it as the reciprocal and use vg.Length.

LongFormat string // longer format
WatchFormat string // show long format when WatchFormat changes between ticks
ShortFormat string // incremental format, shorter
TimeWindow time.Duration // interval for ticks
Copy link
Copy Markdown
Member

@sbinet sbinet Sep 12, 2016

Choose a reason for hiding this comment

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

not sure why any of these fields are exported.
couldn't they just be private?

Copy link
Copy Markdown
Author

@abourget abourget Sep 12, 2016

Choose a reason for hiding this comment

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

yeah they could.. do you think it'd be useful to let people tweak certain rules ? change the formatting..

right now, there's zero internationalization of the labels, etc.. like d3-times scales, it's just English.. I don't currently mind.. but it's very hard-coded..

Copy link
Copy Markdown
Member

@kortschak kortschak Sep 12, 2016

Choose a reason for hiding this comment

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

Unexport them for now.

}

// Ticks implements plot.Ticker.
func (t AutoUnixTimeTicks) Ticks(min, max float64) []Tick {
width := t.Width
if width == 0 {
width = 4 * vg.Inch
Copy link
Copy Markdown
Member

@sbinet sbinet Sep 12, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

pet peeve: could this be (a round value) in centimeters instead of inches?
(gonum in general is trying to follow SI units)

Copy link
Copy Markdown
Member

@kortschak kortschak Sep 12, 2016

Choose a reason for hiding this comment

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

Yes, please.

Copy link
Copy Markdown
Author

@abourget abourget Sep 29, 2016

Choose a reason for hiding this comment

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

I don't mind changing the default value to 10 * vg.Centimer .. the user can however specify that how he wishes in the Width field on the AutoUnixTimeTicks (which is a vg.Length).

The other place where inches are used is in conjunction with DurationPerInch, and these are used solely in internal definitions. I come from Canada where we use SI everywhere. But I think an inch is about the right spacing between ticks, so defining the values like:

    {time.Millisecond, "15:04:05.000", "15:04:05", ".000", time.Millisecond},
    {200 * time.Millisecond, "15:04:05.000", "15:04:05", ".000", 200 * time.Millisecond},
    {500 * time.Millisecond, "15:04:05.000", "15:04:05", ".000", 500 * time.Millisecond},

seems more fitting.. where 500 * time.Millisecond is per inch.. it could be DurationPer2dot5Centimer .. just more awkward :) We'd need to redefine all those rules to some other values.. and use either Centimer or Decimeter.. both of which don't align easily on ticks. Finding visually appealing values for those rules is also tricky.. so I borrowed a bunch of them from d3.. recomputing and realigning them would be a pain, only to internally not use inches.

Are you okay with keeping inches for the rules ?

}

minT := time.Unix(int64(min), 0).UTC()
Copy link
Copy Markdown
Member

@kortschak kortschak Sep 12, 2016

Choose a reason for hiding this comment

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

There is no need for the T suffix, just min and max.

Copy link
Copy Markdown
Author

@abourget abourget Sep 29, 2016

Choose a reason for hiding this comment

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

It's there to distinguish from the function parameters min and max, shortly indicating they're time.Time instead of a float64. Would you prefer some other names ?

maxT := time.Unix(int64(max), 0).UTC()
durationPerInch := maxT.Sub(minT) / time.Duration(width/vg.Inch)

rule := tickIntervals[len(tickIntervals)-1]
for idx := range tickIntervals[:len(tickIntervals)-1] {
Copy link
Copy Markdown
Member

@kortschak kortschak Sep 12, 2016

Choose a reason for hiding this comment

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

Get the value here so you have it by the time you get to L596.

if durationPerInch < tickIntervals[idx+1].DurationPerInch {
rule = tickIntervals[idx]
break
}
}

ticks := []Tick{
Copy link
Copy Markdown
Member

@sbinet sbinet Sep 12, 2016

Choose a reason for hiding this comment

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

replace with just:

var ticks []Tick

Copy link
Copy Markdown
Author

@abourget abourget Sep 12, 2016

Choose a reason for hiding this comment

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

yeah, more left overs :)

// {Value: min, Label: minT.Format(rule.BeginFormat)},
}

monthsDelta := time.Month(rule.TimeWindow / aMonth)
Copy link
Copy Markdown
Member

@kortschak kortschak Sep 12, 2016

Choose a reason for hiding this comment

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

s/monthsDelta/delta/g


//fmt.Println("Months delta", int(monthsDelta))
Copy link
Copy Markdown
Member

@kortschak kortschak Sep 12, 2016

Choose a reason for hiding this comment

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

Delete.

start := minT.Truncate(rule.TimeWindow)
count := 0
Copy link
Copy Markdown
Member

@kortschak kortschak Sep 12, 2016

Choose a reason for hiding this comment

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

var n int

lastWatch := ""
Copy link
Copy Markdown
Member

@kortschak kortschak Sep 12, 2016

Choose a reason for hiding this comment

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

var last string

for {
count++
if monthsDelta > 0 {
// Count in Months now
Copy link
Copy Markdown
Member

@kortschak kortschak Jun 28, 2017

Choose a reason for hiding this comment

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

Full stop.

start = time.Date(start.Year(), start.Month()+monthsDelta, 1, 0, 0, 0, 0, time.UTC)
} else {
start = start.Add(rule.TimeWindow)
}

if count > 20 {
os.Exit(0)
Copy link
Copy Markdown
Member

@sbinet sbinet Sep 12, 2016

Choose a reason for hiding this comment

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

that's pretty ugly :)
at the very least, this should be a panic("plot: run away procedure").

Copy link
Copy Markdown
Member

@sbinet sbinet Sep 12, 2016

Choose a reason for hiding this comment

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

perhaps just return nil and document?

Copy link
Copy Markdown
Author

@abourget abourget Sep 12, 2016

Choose a reason for hiding this comment

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

woohaahh! that was just for debugging :) sorry it went through.. should just take that out.. if you have a sane time range, you won't be hitting that.

}

if start.Before(minT) {
continue
}
if start.After(maxT) {
break
}

label := ""
Copy link
Copy Markdown
Member

@kortschak kortschak Sep 12, 2016

Choose a reason for hiding this comment

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

var label string

newWatch := start.Format(rule.WatchFormat)
Copy link
Copy Markdown
Member

@kortschak kortschak Sep 13, 2016

Choose a reason for hiding this comment

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

What is the meaning of "watch" here?

Copy link
Copy Markdown
Author

@abourget abourget Sep 29, 2016

Choose a reason for hiding this comment

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

watch is probably not the right word... it is the format we check for changes, to decide if we display the long form (like when we change from Sep 1 to Sep 2 when showing hours), or the short (like 11pm, then Sep 2, 12pm when watch changes)

If you look into tests, you'll see there are 2 sorts of labels per graph.. long and short forms.

if lastWatch == newWatch {
label = start.Format(rule.ShortFormat)
} else {
//TODO: overwrite the first tick with the long form if we
Copy link
Copy Markdown
Member

@kortschak kortschak Jun 28, 2017

Choose a reason for hiding this comment

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

s|//TODO: o|// TODO(name): O|

//haven't shown a lonform at all.. instead of always
//showing the longform first.
label = start.Format(rule.LongFormat)
}
lastWatch = newWatch

ticks = append(ticks, Tick{
Value: float64(start.UnixNano()) / float64(time.Second),
Label: label,
})

Copy link
Copy Markdown
Member

@kortschak kortschak Sep 12, 2016

Choose a reason for hiding this comment

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

Delete blank line.

}

return ticks
}

// A Tick is a single tick mark on an axis.
type Tick struct {
// Value is the data value marked by this Tick.
Expand Down
177 changes: 177 additions & 0 deletions axis_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,13 @@
package plot

import (
"fmt"
"math"
"reflect"
"testing"
"time"

"github.com/gonum/plot/vg"
)

func TestAxisSmallTick(t *testing.T) {
Expand Down Expand Up @@ -59,3 +63,176 @@ func labelsOf(ticks []Tick) []string {
}
return labels
}

func allLabelsOf(ticks []Tick) []string {
var labels []string
for _, t := range ticks {
labels = append(labels, t.Label)
}
return labels
}

func TestAutoUnixTimeTicks(t *testing.T) {
d := AutoUnixTimeTicks{Width: 4 * vg.Inch}
for _, test := range []struct {
min, max string
want []string
}{
{
min: "2016-01-01 12:56:30",
max: "2016-01-01 12:56:31",
want: []string{"12:56:30.200", ".400", ".600", ".800", "12:56:31.000"},
},
{
min: "2016-01-01 12:56:01",
max: "2016-01-01 12:56:59",
want: []string{"12:56:05", ":10", ":15", ":20", ":25", ":30", ":35", ":40", ":45", ":50", ":55"},
},
{
min: "2016-01-01 12:56:30",
max: "2016-01-01 12:57:29",
want: []string{"12:56:35", ":40", ":45", ":50", ":55", "12:57:00", ":05", ":10", ":15", ":20", ":25"},
},
{
min: "2016-01-01 12:01:05",
max: "2016-01-01 12:07:00",
want: []string{"12:02:00", "12:03:00", "12:04:00", "12:05:00", "12:06:00", "12:07:00"},
},
{
min: "2016-01-01 12:01:05",
max: "2016-01-01 12:17:00",
want: []string{"Jan 01, 12:02pm", "12:04pm", "12:06pm", "12:08pm", "12:10pm", "12:12pm", "12:14pm", "12:16pm"},
},
{
min: "2016-01-01 12:01:05",
max: "2016-01-01 12:28:00",
want: []string{"Jan 01, 12:05pm", "12:10pm", "12:15pm", "12:20pm", "12:25pm"},
},
{
min: "2016-01-01 12:01:05",
max: "2016-01-01 12:35:00",
want: []string{"Jan 01, 12:05pm", "12:10pm", "12:15pm", "12:20pm", "12:25pm", "12:30pm", "12:35pm"},
},
{
min: "2016-01-01 12:01:05",
max: "2016-01-01 12:40:00",
want: []string{"Jan 01, 12:05pm", "12:10pm", "12:15pm", "12:20pm", "12:25pm", "12:30pm", "12:35pm", "12:40pm"},
},
{
min: "2016-01-01 12:01:05",
max: "2016-01-01 12:45:00",
want: []string{"Jan 01, 12:05pm", "12:10pm", "12:15pm", "12:20pm", "12:25pm", "12:30pm", "12:35pm", "12:40pm", "12:45pm"},
},
{
min: "2016-01-01 12:01:05",
max: "2016-01-01 13:05:00",
want: []string{"Jan 01, 12:15pm", "12:30pm", "12:45pm", "1:00pm"},
},
{
min: "2016-01-01 12:01:05",
max: "2016-01-01 13:05:00",
want: []string{"Jan 01, 12:15pm", "12:30pm", "12:45pm", "1:00pm"},
},
{
min: "2016-01-01 12:01:05",
max: "2016-01-01 16:05:00",
want: []string{"Jan 1, 1pm", "2pm", "3pm", "4pm"},
},
{
min: "2016-01-01 20:01:05",
max: "2016-01-02 07:59:00",
want: []string{"Jan 1, 9pm", "10pm", "11pm", "Jan 2, 12am", "1am", "2am", "3am", "4am", "5am", "6am", "7am"},
},
{
min: "2016-01-01 12:01:05",
max: "2016-01-02 13:59:00",
want: []string{"Jan 1, 6pm", "Jan 2, 12am", "6am", "12pm"},
},
{
min: "2016-01-01 12:01:05",
max: "2016-01-04 13:59:00",
want: []string{"Jan 2", "12pm", "Jan 3", "12pm", "Jan 4", "12pm"},
},
{
min: "2016-01-01 12:01:05",
max: "2016-01-06 13:59:00",
want: []string{"Jan 2", "3", "4", "5", "6"},
},
{
min: "2016-01-01 12:01:05",
max: "2016-01-09 13:59:00",
want: []string{"Jan 2", "4", "6", "8"},
},
{
min: "2016-01-01 12:01:05",
max: "2016-01-25 13:59:00",
want: []string{"Jan 2", "4", "6", "8", "10", "12", "14", "16", "18", "20", "22", "24"},
},
{
min: "2016-01-01 12:01:05",
max: "2016-02-06 13:59:00",
want: []string{"Jan 4", "11", "18", "25", "Feb 1"},
},
{
min: "2016-01-01 12:01:05",
max: "2016-02-28 13:59:00",
want: []string{"Jan 4", "11", "18", "25", "Feb 1", "8", "15", "22"},
},
{
min: "2016-01-01 12:01:05",
max: "2016-04-28 13:59:00",
want: []string{"Jan 4", "11", "18", "25", "Feb 1", "8", "15", "22", "29", "Mar 7", "14", "21", "28", "Apr 4", "11", "18", "25"},
},
{
min: "2016-01-01 12:01:05",
max: "2016-09-28 13:59:00",
want: []string{"Feb 2016", "Mar", "Apr", "May", "Jun", "Jul", "Aug", "Sep"},
},
{
min: "2016-01-01 12:01:05",
max: "2016-12-28 13:59:00",
want: []string{"Feb 2016", "Mar", "Apr", "May", "Jun", "Jul", "Aug", "Sep", "Oct", "Nov", "Dec"},
},
{
min: "2016-01-01 12:01:05",
max: "2017-02-28 13:59:00",
want: []string{"Feb 2016", "May", "Aug", "Nov", "Feb 2017"},
},
{
min: "2016-01-01 12:01:05",
max: "2017-08-28 13:59:00",
want: []string{"Feb 2016", "May", "Aug", "Nov", "Feb 2017", "May", "Aug"},
},
{
min: "2016-01-01 12:01:05",
max: "2018-08-28 13:59:00",
want: []string{"Feb 2016", "Aug", "Feb 2017", "Aug", "Feb 2018", "Aug"},
},
{
min: "2016-01-01 12:01:05",
max: "2020-08-28 13:59:00",
want: []string{"2016", "2017", "2018", "2019", "2020"},
},
{
min: "2016-01-01 12:01:05",
max: "2048-08-28 13:59:00",
want: []string{"2017", "2022", "2027", "2032", "2037", "2042", "2047"},
},
} {
fmt.Println("For dates", test.min, test.max)
Copy link
Copy Markdown
Member

@sbinet sbinet Sep 12, 2016

Choose a reason for hiding this comment

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

remove this line

ticks := d.Ticks(dateToFloat64(test.min), dateToFloat64(test.max))
got := allLabelsOf(ticks)
if !reflect.DeepEqual(got, test.want) {
t.Errorf("tick labels mismatch:\ndate1: %s\ndate2: %s\ngot: %#v\nwant:%q", test.min, test.max, got, test.want)
}
}
}

func dateToFloat64(date string) float64 {
t, err := time.Parse("2006-01-02 15:04:05", date)
if err != nil {
panic(err)
}

return float64(t.UTC().UnixNano()) / float64(time.Second)
}