You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
test_that("it sets the x_axis_breaks correctly", {
m <- mock()
stub(ptd_create_ggplot, "scale_x_datetime", m)
set.seed(123)
d <- data.frame(x = to_datetime("2020-01-01") + 1:20, y = rnorm(20))
...
The tests below this one use as.Date(), which works fine. Do we want a date, or a datetime?
I can't see why the to_datetime.character Method wouldn't work on "2020-01-01". It should just call as.POSIXct().
Could we just use lubridate::as_datetime() throughout the package, instead of the internal to_datetime()? I think that function will handle the different cases that to_datetime() tries to handle - but perhaps there is a nuance here about exactly what the output needs to be. (I haven't gone into learning the differences between POSIXt and POSIXct!)
[lubridate is now in the core tidyverse so it doesn't feel too unreasonable to add it as a dependency, given we already have dplyr and stringr and ggplot2 of course.]
Does this particular test need to_datetime, or would as.Date() or lubridate::as_date/as_datetime work just as well?
For now I am just going to edit the test to use as.Date() instead.
Here's a very simple reprex:
NHSRplotthedots:::to_datetime("2020-01-01")
#> Error in UseMethod("to_datetime"): no applicable method for 'to_datetime' applied to an object of class "character"
as.Date("2020-01-01")
#> [1] "2020-01-01"
as.POSIXct("2020-01-01")
#> [1] "2020-01-01 GMT"lubridate::as_datetime("2020-01-01")
#> [1] "2020-01-01 UTC"
including {lubridate} would be a last resort - it's only needed in one place, and including an extra dependency has issues (it's advised you stay below something like 20 dependencies when submitting to CRAN)
There's a strange error in one of the tests.
If you look at /tests/testthat/test-ptd_create_ggplot.R#L109
The tests below this one use
as.Date()
, which works fine. Do we want a date, or a datetime?I can't see why the
to_datetime.character
Method wouldn't work on "2020-01-01". It should just callas.POSIXct()
.Could we just use
lubridate::as_datetime()
throughout the package, instead of the internalto_datetime()
? I think that function will handle the different cases thatto_datetime()
tries to handle - but perhaps there is a nuance here about exactly what the output needs to be. (I haven't gone into learning the differences between POSIXt and POSIXct!)[
lubridate
is now in the core tidyverse so it doesn't feel too unreasonable to add it as a dependency, given we already have dplyr and stringr and ggplot2 of course.]Does this particular test need
to_datetime
, or wouldas.Date()
orlubridate::as_date/as_datetime
work just as well?For now I am just going to edit the test to use
as.Date()
instead.Here's a very simple reprex:
@tomjemmett
Related: #169 ?
The text was updated successfully, but these errors were encountered: