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

Preserve milliseconds with dynamicTicks #1871

Merged
merged 5 commits into from
Dec 8, 2020

Conversation

FlukeAndFeather
Copy link
Contributor

Resolves #1870

Adds an ISO 8601 time_format with milliseconds to to_JSON(). So the R objects don't change (i.e. POSIXct objects remain POSIXct) but their conversion to JSON does.

@cpsievert
Copy link
Collaborator

cpsievert commented Oct 26, 2020

Thank you for the contribution! Would you mind providing some more details as to why this works? That is, time_format eventually gets passed to what function? Also, how did you land on this specific format?

@FlukeAndFeather
Copy link
Contributor Author

Sure thing. The root of the bug is how POSIXct objects are written to JSON when dynamicTicks = TRUE. Without dynamicTicks, POSIXct values are converted to numeric, so the JSON conversion retains the sub-second accuracy (see x1 in the code block below). With dynamicTicks, the x-axis remains POSIXct and ultimately is converted to text. However, by default, the conversion from POSIXct to character drops milliseconds (see x2). The function that does the conversion is jsonlite::toJSON(), called from to_JSON() in utils.R:1023. Specifying the time_format argument in jsonlite::toJSON() replaces the default POSIXct-to-character behavior. Weirdly, this functionality is undocumented but it's mentioned in other places (coincidentally, your comment on another issue) and you can see exactly how it works here.

This specific time format is ISO 8601 compliant, so it should be universally readable. It specifies the milliseconds (which fixes this bug) and the time zone (which may be useful depending on future development of plotly).

Hope this helps. Please let me know if there are any other questions.

library(ggplot2)
library(plotly)
#> 
#> Attaching package: 'plotly'
#> The following object is masked from 'package:ggplot2':
#> 
#>     last_plot
#> The following object is masked from 'package:stats':
#> 
#>     filter
#> The following object is masked from 'package:graphics':
#> 
#>     layout

# Create a sine curve with sub-second precision
d <- data.frame(x = as.POSIXct("1970-01-01", tz = "UTC") + (0:999) / 10, 
                y = sin((0:999) * 8 * pi / 1000))
p <- ggplot(d, aes(x, y)) +
    geom_line()

# plotly without dynamicTicks - x converted to numeric
b1 <- plotly_build(ggplotly(p))
class(b1$x$data[[1]]$x)
#> [1] "numeric"
b1$x$data[[1]]$x[1:5]
#> [1] 0.0 0.1 0.2 0.3 0.4
j1 <- plotly_json(b1, jsonedit = FALSE)
x1 <- unlist(jsonlite::parse_json(j1)$data[[1]]$x)
x1[1:5]
#> [1] 0.0 0.1 0.2 0.3 0.4
# plotly with dynamicTicks - x remains POSIXct
b2 <- plotly_build(ggplotly(p, dynamicTicks = TRUE))
class(b2$x$data[[1]]$x)
#> [1] "POSIXct" "POSIXt"
b2$x$data[[1]]$x[1:5]
#> [1] "1970-01-01 00:00:00 UTC" "1970-01-01 00:00:00 UTC"
#> [3] "1970-01-01 00:00:00 UTC" "1970-01-01 00:00:00 UTC"
#> [5] "1970-01-01 00:00:00 UTC"
j2 <- plotly_json(b2, jsonedit = FALSE)
x2 <- unlist(jsonlite::parse_json(j2)$data[[1]]$x)
x2[1:5]
#> [1] "1970-01-01 00:00:00" "1970-01-01 00:00:00" "1970-01-01 00:00:00"
#> [4] "1970-01-01 00:00:00" "1970-01-01 00:00:00"

Created on 2020-10-26 by the reprex package (v0.3.0)

@cpsievert
Copy link
Collaborator

cpsievert commented Oct 26, 2020

Thank you, that helps a lot! It looks like this format is slight different from the format that plotly.js expects. Would you mind looking into how to get things into that ('yyyy-mm-dd HH:MM:SS.ssssss') format?

> x2[1:5]
[1] "1970-01-01T00:00:00.000+0000" "1970-01-01T00:00:00.100+0000"
[3] "1970-01-01T00:00:00.200+0000" "1970-01-01T00:00:00.300+0000"
[5] "1970-01-01T00:00:00.400+0000"

According to plotly documentation (https://plotly.com/chart-studio-help/date-format-and-time-series/): "Chart Studio’s date format is 'yyyy-mm-dd HH:MM:SS.ssssss'.", so time_format in jsonlite::toJSON() changed to match.
@FlukeAndFeather
Copy link
Contributor Author

Done and done.

@FlukeAndFeather
Copy link
Contributor Author

Sorry if this is a bother. I'm not super familiar with Travis CI, so I don't know if the error below is an issue with my code or with Travis. Hitting some kind of issue on the release version.

The command "if [[ "$TRAVIS_R_VERSION_STRING" == "release" ]]; then docker run -e GITHUB_PAT=$GITHUB_PAT -e MAPBOX_TOKEN=$MAPBOX_TOKEN -e VMODE="ci" -v $(pwd):/home/plotly --privileged cpsievert/plotly-orca; fi" failed and exited with 1 during .

@FlukeAndFeather
Copy link
Contributor Author

I think the Travis issue was due to a visual test not passing. I updated the test image for line-milliseconds.svg following the instructions in CONTRIBUTING.md. But since that's not a code change I don't think a new Travis build check was triggered.

@FlukeAndFeather
Copy link
Contributor Author

Ah, I was wrong. Travis was triggered and all test pass. I think this pull request is ready for merging now.

@cpsievert cpsievert added this to the CRAN 4.9.3 milestone Nov 17, 2020
@cpsievert
Copy link
Collaborator

Thank you!

@cpsievert cpsievert merged commit ea7a344 into plotly:master Dec 8, 2020
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.

dynamicTicks aggregates sub-second values
2 participants