Skip to content

Conversation

@kostya-sh
Copy link
Contributor

DO NOT MERGE.

This change is for review and discussion only.

This is backward incompatible change.

DO NOT MERGE.

This change is for review and discussion only.

This is backward incompatible change.
@kortschak
Copy link
Member

I like this.

func (utt UnixTimeTicks) Ticks(min, max float64) []Tick {
if utt.Ticker == nil {
utt.Ticker = DefaultTicks{}
func (utt UnixTimeLabeler) Labels(ticks []Tick) []string {
Copy link
Member

Choose a reason for hiding this comment

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

perhaps s/utt/utl/g ?

@sbinet
Copy link
Member

sbinet commented Jun 3, 2016

looks good.
I may even abandon #278 in favor of this change.

@kostya-sh
Copy link
Contributor Author

If the agreement is that the new API is an improvement over the current API and will be merged I am happy to finsh this PR (it requires some cleanup, more docs and tests).

I also found previous discussion about tick generation (https://groups.google.com/forum/#!topic/gonum-dev/g7ZgAVItbbA) and https://github.com/vdobler/plot-1/blob/redesign/design.md by @vdobler. There are a few proposed features. Maybe it is worth checking how to implement them (and if it is possible at all) with the new API?

@sbinet
Copy link
Member

sbinet commented Jun 20, 2016

(apologies for the belated answer)

yes, I guess it would be worth to get a sense of how this might look like.
(I myself have still to update my PR #245 and/or provide a more theme-based oriented facility, like https://godoc.org/golang.org/x/exp/shiny/widget/theme#Theme)

@eaburns
Copy link
Member

eaburns commented Oct 25, 2016

Any news on this? We are trying to clean up our PRs. Folks seemed to like this idea, but it still has some outstanding comments. Do you plan to address them? If not, we plan to close this soon.

@kostya-sh
Copy link
Contributor Author

I am going to close this PR. I am still not sure if the proposed interface is flexible enough. Additionally I do not have much time to work on this at the moment.

@kostya-sh kostya-sh closed this Oct 25, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants