Skip to content

Commit

Permalink
Only support hours without zero-padding
Browse files Browse the repository at this point in the history
Remove support for non-zero-padded minutes and seconds. The ISO 8601
standard requires all time components be zero-padded. That said, we
do support hours that aren't zero-padded, for convenience.

Also make the validation regex stricter. The first digit for minutes
and seconds cannot be > 5. Separate regex into multiple pieces, to
make them easier to understand later.

See joshuaulrich#326. See joshuaulrich#327.
  • Loading branch information
joshuaulrich committed Sep 6, 2020
1 parent da44bca commit 097c718
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 8 deletions.
18 changes: 13 additions & 5 deletions R/xts.methods.R
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,19 @@

.subsetTimeOfDay <- function(x, fromTimeString, toTimeString) {
validateTimestring <- function(time) {
if (isFALSE(grepl(paste0("^(([0-9]{1,2}|",
"[0-9]{1,2}:[0-9]{1,2})$)|",
"([0-9]{1,2}:[0-9]{1,2}:[0-9]{1,2}($|?(.[0-9]+)))"),
time)))
stop("Supply time-of-day subsetting in the format of T%H:%M:%OS/T%H:%M:%OS")
h <- "(?:[01]?\\d|2[0-3])"
hm <- paste0(h, "(?::[0-5]\\d)")
hms <- paste0(hm, "(?::[0-5]\\d)")
hmsS <- paste0(hms, "(?:\\.\\d{1,9})?")
pattern <- paste(h, hm, hms, hmsS, sep = ")$|^(")
pattern <- paste0("^(", pattern, "$)")

if (!grepl(pattern, time)) {
# FIXME: this isn't necessarily true...
# colons aren't required, and neither are all of the components
stop("Supply time-of-day subsetting in the format of T%H:%M:%OS/T%H:%M:%OS",
call. = FALSE)
}
}
padTimestring <- function(time) {
ifelse(grepl("^[0-9]{1,2}$", time), paste0(time, ":00"), time)
Expand Down
7 changes: 4 additions & 3 deletions inst/unitTests/runit.subset-time-of-day.R
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,10 @@ test.time_of_day_by_second <- function() {
983L, 984L, 985L, 986L)])

checkIdentical(.index(x["T01:58:05/T02:01:09"]), i1)
# Can omit 0 padding, as is possible by default with as.POSIXct():
checkIdentical(.index(x["T01:58:5/T02:1:9"]), i1)
checkIdentical(.index(x["T01:58:05.000/T02:01:09.000"]), i1)
# Can only omit 0 padding for hours. Only for convenience because it does
# not conform to the ISO 8601 standard, which requires padding with zeros.
checkIdentical(.index(x["T1:58:05/T2:01:09"]), i1)
checkIdentical(.index(x["T1:58:05.000/T2:01:09.000"]), i1)
}

test.time_of_day_end_before_start <- function() {
Expand Down

0 comments on commit 097c718

Please sign in to comment.