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.

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

Choice to fixe the data timezone or not... #16

Closed
wants to merge 17 commits into from
Closed

Choice to fixe the data timezone or not... #16

wants to merge 17 commits into from

Conversation

bthieurmel
Copy link
Contributor

Dygraphs sticks to the timezone of the machine. In certain cases, among whom most with people with whom I work, we wish to fix the timezone of the data, about are the machine, places....
We can do that using moment.js and moment-timezone.js, and a appropriate dygprahs.js with custom valueFormatter, axisLabelFormatter, and ticker.
I do quite the same things like
http://stackoverflow.com/questions/24196183/dygraph-showing-dates-in-an-arbitrary-timezone/24196184#24196184
I think it can be very usefull.

Try this
devtools::install_github(c("bthieurmel/dygraphs"))
mytz = "Indian/Mahe"
mytz = "UTC"

n=300
don <- data.frame(date = seq(c(ISOdate(2000,3,20)), by = "10 min", length.out = n), a = rnorm(n,100, 100), b = rnorm(n,100, 100))
attr(don[,1],"tzone") <- mytz
don <- xts(don[,-1], order.by=don[,1])

d = dygraph(don, main ="Example dygraph", ylab = "Random data", fixed.tz.data = TRUE) %>%
dyRangeSelector(keepMouseZoom = TRUE) %>%
dyLegend(show = "always",labelsSeparateLines = TRUE)

d

saveWidget(d, "widget.html")
saveWidget(d, "widget2.html", selfcontained = FALSE)

For let the choice to fixe the timezone of the graphic
For let the choice to fixe the timezone of the graphic
For let the choice to fixe the timezone of the graphic
For let the choice to fixe the timezone of the graphic
@@ -30,8 +30,18 @@ HTMLWidgets.widget({
x.attrs.legend = "always";
}

// set appropriated function in case of fixed tz
if (attrs.axes.x.axisLabelFormatter === undefined & x.fixedtz)
Copy link
Member

Choose a reason for hiding this comment

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

It looks like this is a bitwise and (&). Do we want logical and (&&) instead? (same applies to the comparisons immediately below)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're probably right. I'm not a good javascript coder...

Copy link
Member

Choose a reason for hiding this comment

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

Maybe also add parens around the first condition just for clarity

@jjallaire
Copy link
Member

This is a fantastic PR! Thanks so much for taking the initiative to do this. When building the package I ran into the timezone issue and ascertained that some integration of moment would be necessary but I wasn't quite sure how this could/should work.

We are actually about to announce the dygraphs package (tomorrow) so I don't want to take this PR right on the threshold of having many more users installing and kicking the tires (just in case any bugs are introduced). Let's clean things up and then I'll merge it in a couple of weeks.

@bthieurmel
Copy link
Contributor Author

Perfect. I am very satisfied if this new feature can make part of dygraphs package in the next weeks. I must be able to look at some points above tomorrow.

@jjallaire
Copy link
Member

Thanks, much appreciated!

@bthieurmel
Copy link
Contributor Author

Hi,
I'm just doiing modifications :

  • add parens and replace & by && in dygraphs.js
  • move fixed.tz.data argument from dygraph.R to options.R, and rename it as fixedDataTimezone, (and update dyOptions.Rd...)

So for testing is now :

devtools::install_github(c("bthieurmel/dygraphs"))

require(dygraphs)
require(xts)

mytz = "Indian/Mahe"
#mytz = "UTC"

n=300
don <- data.frame(date = seq(c(ISOdate(2000,3,20)), by = "10 min", length.out = n), a = rnorm(n,100, 100), b = rnorm(n,100, 100))
attr(don[,1],"tzone") <- mytz
don <- xts(don[,-1], order.by=don[,1])

d = dygraph(don, main ="Example dygraph", ylab = "Random data") %>%
dyOptions(fixedDataTimezone = TRUE) %>%
dyRangeSelector(keepMouseZoom = TRUE) %>%
dyLegend(show = "always",labelsSeparateLines = TRUE)

d

saveWidget(d, "widget.html")

@jjallaire
Copy link
Member

Looks good, I'll pull this in a couple of weeks!

On Wed, Dec 17, 2014 at 3:55 AM, Thieurmel notifications@github.com wrote:

Hi,
I'm just doiing the little modifications :

  • add parens and replace & by && in dygraphs.js
  • move fixed.tz.data argument from dygraph.R to options.R,


Reply to this email directly or view it on GitHub
#16 (comment).

@jjallaire
Copy link
Member

I'm wondering if pulling in moment.js actually provides us with the ability to implement the following as time zone options:

  • utc (show in UTC)
  • data (show in whatever timezone the data series is using -- presumably the local one on the server?)
  • client (show using the browser's time-zone)

Or perhaps better couched as: utc, server, client ?

jjallaire pushed a commit that referenced this pull request Dec 19, 2014
@jjallaire
Copy link
Member

I've rebased these changes and merged them onto the v0.3 branch (see: https://github.com/rstudio/dygraphs/compare/v0.3) which will be merged into master sometime in early January. I'll therefore close this PR for now (if you have other changes at any ponit just fork the v0.3 branch)

@jjallaire jjallaire closed this Dec 19, 2014
@jjallaire jjallaire reopened this Dec 21, 2014
@jjallaire
Copy link
Member

Noticed a couple of things I'd like to see cleaned up before taking the PR:

  1. The custom formatter ends up printing labels on two lines and in that case the bottom of the label is clipped off. See:

rplot

  1. In the legend the full name of the timezone is printed (e.g. "America/Havana"). This should be some type of abbreviation of the timezone instead.

@bthieurmel
Copy link
Contributor Author

Indeed. I don't know what it's easily possible to make for shorter names of timezone... We can also perhaps add an option : enabled or not showing timezone or for choose timezone location ? I can look at it in 2015, after Christmas.

@jjallaire
Copy link
Member

Okay, sounds good! (btw let's continue working on your PR branch rather
than the v0.3 branch that I mentioned previously)

On Mon, Dec 22, 2014 at 8:10 AM, Thieurmel notifications@github.com wrote:

Indeed. I don't know what it's easily possible to make for shorter names
of timezone... We can also perhaps add an option : enabled or not showing
timezone or for choose timezone location ? I can look at it in 2015, after
Christmas.


Reply to this email directly or view it on GitHub
#16 (comment).

@bthieurmel
Copy link
Contributor Author

Hi. I think it's good with some little modifications done in the last commit :

  • change return mmnt.format('MMM YYYY') to return mmnt.format('MMM YY') in dygraphs.js, line 236
  • change tz to mmnt.zoneAbbr() in dygraphs.js, line 259, 261, 263, 265.

@jjallaire
Copy link
Member

Merged via ad3b9d7 (some small cleanup and other changes made including renaming the option to useDataTimezone)

@jjallaire jjallaire closed this Jan 5, 2015
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.

None yet

2 participants