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

naive_time_parse() can fail when rounding up to 60 seconds #230

Closed
DavisVaughan opened this issue May 7, 2021 · 2 comments · Fixed by #231
Closed

naive_time_parse() can fail when rounding up to 60 seconds #230

DavisVaughan opened this issue May 7, 2021 · 2 comments · Fixed by #231

Comments

@DavisVaughan
Copy link
Member

library(clock)

# Round up to 60 seconds (looks invalid)? That causes the NA?
naive_time_parse("2021-04-16T22:35:59.955Z", format = "%Y-%m-%dT%H:%M:%6SZ", precision = "second")

# Rounds up to a valid 59 seconds
naive_time_parse("2021-04-16T22:35:58.955Z", format = "%Y-%m-%dT%H:%M:%6SZ", precision = "second")

# No rounding
naive_time_parse("2021-04-16T22:35:59.955Z", format = "%Y-%m-%dT%H:%M:%6SZ", precision = "millisecond")
@DavisVaughan
Copy link
Member Author

HowardHinnant/date#667

@DavisVaughan
Copy link
Member Author

DavisVaughan commented May 7, 2021

Possibly solve by instead parsing into microseconds, and then giving the user the option of how to round. Advise them to use a real clock type if they have more precise date-times than microsecond that they need to parse, or if they want to keep the fractional seconds.

Consider how this should work for other date-time parsers, and for the date parser.

library(clock)
library(rlang)

date_time_parse <- function(x,
                            zone,
                            ...,
                            format = NULL,
                            locale = clock_locale(),
                            fractional = "floor",
                            nonexistent = NULL,
                            ambiguous = NULL) {
  x <- naive_time_parse(x, ..., format = format, precision = "microsecond", locale = locale)
  rounder <- get_rounder(fractional)
  x <- rounder(x, "second")
  as.POSIXct(x, tz = zone, nonexistent = nonexistent, ambiguous = ambiguous)
}

get_rounder <- function(fractional) {
  fractional <- arg_match0(fractional, c("cast", "round", "floor", "ceiling"), "fractional")
  
  switch (
    fractional,
    cast = time_point_cast,
    round = time_point_round,
    floor = time_point_floor,
    ceiling = time_point_ceiling
  )
}

drive_modified_time <- c(
  "2021-04-16T22:36:30.686Z",
  "2021-04-16T22:35:59.955Z",
  "2021-04-16T22:35:34.369Z"
)

# this continues to work
date_time_parse(drive_modified_time, zone = "UTC", format = "%Y-%m-%dT%H:%M:%6SZ")
#> [1] "2021-04-16 22:36:30 UTC" "2021-04-16 22:35:59 UTC"
#> [3] "2021-04-16 22:35:34 UTC"

# this now works
date_time_parse(drive_modified_time, zone = "UTC", format = "%Y-%m-%dT%H:%M:%SZ")
#> [1] "2021-04-16 22:36:30 UTC" "2021-04-16 22:35:59 UTC"
#> [3] "2021-04-16 22:35:34 UTC"

# can control rounding method
date_time_parse(drive_modified_time, zone = "UTC", format = "%Y-%m-%dT%H:%M:%SZ", fractional = "round")
#> [1] "2021-04-16 22:36:31 UTC" "2021-04-16 22:36:00 UTC"
#> [3] "2021-04-16 22:35:34 UTC"

# this is unsupported
date_time_parse(
  "2021-04-16T22:36:30.1234567Z",
  zone = "UTC", 
  format = "%Y-%m-%dT%H:%M:%SZ"
)
#> Warning: Failed to parse 1 string at location 1. Returning `NA` at that
#> location.
#> [1] NA

# technically can do this, but leave undocumented
date_time_parse(
  "2021-04-16T22:36:30.1234567Z",
  zone = "UTC", 
  format = "%Y-%m-%dT%H:%M:%10SZ"
)
#> [1] "2021-04-16 22:36:30 UTC"

Created on 2021-05-07 by the reprex package (v1.0.0)

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 a pull request may close this issue.

1 participant