nhs-r-community / NHSRplotthedots

An SPC package to support NHSE/I 'Making Data Count' programme
https://nhs-r-community.github.io/NHSRplotthedots/
Other
48 stars 23 forks source link

to_datetime() doesn't handle POSIXct date-times? #169

Closed francisbarton closed 1 year ago

francisbarton commented 1 year ago

The to_datetime methods are set up to handle Date, POSIXt and character inputs, but if you feed in datetimes in POSIXct (which shares the class 'POSIXt'), to_datetime gives an error. I guess the input really has to be just a date not a date-time.

Relevant to https://github.com/nhs-r-community/NHSRplotthedots/blob/main/R/ptd_spc.R#L129

Can/should a method be added to return identity for POSIXct inputs, as that is the desired output anyway?

x <- tibble::tibble(date = c("2022-06-01", "2022-07-01")) |>
  dplyr::mutate(across(date, as.POSIXct))

x
#> # A tibble: 2 × 1
#>   date               
#>   <dttm>             
#> 1 2022-06-01 00:00:00
#> 2 2022-07-01 00:00:00

# in case conversion from numeric makes any difference (no)
y <- tibble::tibble(date = c(19339, 19340)) |>
  dplyr::mutate(across(date, as.POSIXct.Date))

y
#> # A tibble: 2 × 1
#>   date               
#>   <dttm>             
#> 1 2022-12-13 00:00:00
#> 2 2022-12-14 00:00:00

NHSRplotthedots:::to_datetime(x[["date"]])
#> Error in UseMethod("to_datetime"): no applicable method for 'to_datetime' applied to an object of class "c('POSIXct', 'POSIXt')"
NHSRplotthedots:::to_datetime(y[["date"]])
#> Error in UseMethod("to_datetime"): no applicable method for 'to_datetime' applied to an object of class "c('POSIXct', 'POSIXt')"

Created on 2022-12-14 with reprex v2.0.2

Session info ``` r sessioninfo::session_info() #> ─ Session info ─────────────────────────────────────────────────────────────── #> setting value #> version R version 4.2.2 (2022-10-31 ucrt) #> os Windows 10 x64 (build 19042) #> system x86_64, mingw32 #> ui RTerm #> language (EN) #> collate English_United Kingdom.utf8 #> ctype English_United Kingdom.utf8 #> tz Europe/London #> date 2022-12-14 #> pandoc 2.19.2 @ C:/Users/francis.barton/AppData/Local/Programs/RStudio/bin/quarto/bin/tools/ (via rmarkdown) #> #> ─ Packages ─────────────────────────────────────────────────────────────────── #> package * version date (UTC) lib source #> assertthat 0.2.1 2019-03-21 [1] CRAN (R 4.2.2) #> cli 3.4.1 2022-09-23 [1] CRAN (R 4.2.2) #> colorspace 2.0-3 2022-02-21 [1] CRAN (R 4.2.2) #> DBI 1.1.3 2022-06-18 [1] CRAN (R 4.2.2) #> digest 0.6.31 2022-12-11 [1] CRAN (R 4.2.2) #> dplyr 1.0.10 2022-09-01 [1] CRAN (R 4.2.2) #> evaluate 0.18 2022-11-07 [1] CRAN (R 4.2.2) #> fansi 1.0.3 2022-03-24 [1] CRAN (R 4.2.2) #> fastmap 1.1.0 2021-01-25 [1] CRAN (R 4.2.2) #> fs 1.5.2 2021-12-08 [1] CRAN (R 4.2.2) #> generics 0.1.3 2022-07-05 [1] CRAN (R 4.2.2) #> ggplot2 3.4.0 2022-11-04 [1] CRAN (R 4.2.2) #> glue 1.6.2 2022-02-24 [1] CRAN (R 4.2.2) #> gtable 0.3.1 2022-09-01 [1] CRAN (R 4.2.2) #> highr 0.9 2021-04-16 [1] CRAN (R 4.2.2) #> htmltools 0.5.4 2022-12-07 [1] CRAN (R 4.2.2) #> knitr 1.41 2022-11-18 [1] CRAN (R 4.2.2) #> lifecycle 1.0.3 2022-10-07 [1] CRAN (R 4.2.2) #> magrittr 2.0.3 2022-03-30 [1] CRAN (R 4.2.2) #> munsell 0.5.0 2018-06-12 [1] CRAN (R 4.2.2) #> NHSRplotthedots 0.1.0 2021-11-03 [1] CRAN (R 4.2.2) #> pillar 1.8.1 2022-08-19 [1] CRAN (R 4.2.2) #> pkgconfig 2.0.3 2019-09-22 [1] CRAN (R 4.2.2) #> purrr 0.3.5 2022-10-06 [1] CRAN (R 4.2.2) #> R.cache 0.16.0 2022-07-21 [1] CRAN (R 4.2.2) #> R.methodsS3 1.8.2 2022-06-13 [1] CRAN (R 4.2.0) #> R.oo 1.25.0 2022-06-12 [1] CRAN (R 4.2.0) #> R.utils 2.12.2 2022-11-11 [1] CRAN (R 4.2.2) #> R6 2.5.1 2021-08-19 [1] CRAN (R 4.2.2) #> reprex 2.0.2 2022-08-17 [1] CRAN (R 4.2.2) #> rlang 1.0.6 2022-09-24 [1] CRAN (R 4.2.2) #> rmarkdown 2.18 2022-11-09 [1] CRAN (R 4.2.2) #> rstudioapi 0.14 2022-08-22 [1] CRAN (R 4.2.2) #> scales 1.2.1 2022-08-20 [1] CRAN (R 4.2.2) #> sessioninfo 1.2.2 2021-12-06 [1] CRAN (R 4.2.2) #> stringi 1.7.8 2022-07-11 [1] CRAN (R 4.2.1) #> stringr 1.5.0 2022-12-02 [1] CRAN (R 4.2.2) #> styler 1.8.1 2022-11-07 [1] CRAN (R 4.2.2) #> tibble 3.1.8 2022-07-22 [1] CRAN (R 4.2.2) #> tidyselect 1.2.0 2022-10-10 [1] CRAN (R 4.2.2) #> utf8 1.2.2 2021-07-24 [1] CRAN (R 4.2.2) #> vctrs 0.5.1 2022-11-16 [1] CRAN (R 4.2.2) #> withr 2.5.0 2022-03-03 [1] CRAN (R 4.2.2) #> xfun 0.35 2022-11-16 [1] CRAN (R 4.2.2) #> yaml 2.3.6 2022-10-18 [1] CRAN (R 4.2.1) #> #> ────────────────────────────────────────────────────────────────────────────── ```
tomjemmett commented 1 year ago

Thanks, the issue is going to be in https://github.com/nhs-r-community/NHSRplotthedots/blob/main/R/to_datetime.R not handling that particular class. I thought the to_datetime.POSIXt <- identity method should have been sufficient, but might need to add extra cases

tomjemmett commented 1 year ago

The issue here is with s3 generics being a bit funny if you don't export them, but then try to use with the ::: operator. Internally it works, but when you use NHSRplotthedots:::to_datetime it can't find the method to dispatch to.

Simply flagging all of the methods with @export causes to_datetime(x[["date"]]) to work.

However, this wasn't intended to be a useful function outside of the package, just something to quickly coerce anything to a POSIXct (and let as.POSIXct handle errors). If it was to be exported then we would need to make the code far more rigorous, so I'm going to mark this as closed/wont-fix for now, but happy to hear any counter arguments 🙂

francisbarton commented 1 year ago

Good digging @tomjemmett! No issue with closing this, for the reasons you have outlined.