tidyverse / lubridate

Make working with dates in R just that little bit easier
https://lubridate.tidyverse.org
GNU General Public License v3.0
724 stars 207 forks source link

`hm()` does not ignore NA values in ifelse #1125

Closed lawalter closed 11 months ago

lawalter commented 1 year ago

I believe I found a bug when using lubridate::hm() inside an ifelse() when conditionally asked to ignore NA values.

When running a transformation on regularly formatted times, there is no issue:

tibble::tibble(
  char_time = c("05:00", "10:57", "10:58", "16:50", "21:59"),
  hm_time = lubridate::hm(char_time))

# # A tibble: 5 × 2
# char_time hm_time   
# <chr>     <Period>  
# 1 05:00     5H 0M 0S  
# 2 10:57     10H 57M 0S
# 3 10:58     10H 58M 0S
# 4 16:50     16H 50M 0S
# 5 21:59     21H 59M 0S

When running a transformation with an NA value in the mix, I get the expected warning message:

tibble::tibble(
  char_time = c("05:00", "10:57", "10:58", "16:50", "21:59", NA),
  hm_time = lubridate::hm(char_time))

# # A tibble: 6 × 2
# char_time hm_time   
# <chr>     <Period>  
# 1 05:00     5H 0M 0S  
# 2 10:57     10H 57M 0S
# 3 10:58     10H 58M 0S
# 4 16:50     16H 50M 0S
# 5 21:59     21H 59M 0S
# 6 NA        NA        
# Warning message:
#   In .parse_hms(..., order = "HM", quiet = quiet) :
#   Some strings failed to parse, or all strings are NAs

If asked to ignore NA values for the transformation inside an ifelse() statement, the warning message is still returned. All values expected to be transformed to type "Period" are instead 0 and type "double":

tibble::tibble(
  char_time = c("05:00", "10:57", "10:58", "16:50", "21:59", NA),
  hm_time = ifelse(!is.na(char_time), lubridate::hm(char_time), NA))

# # A tibble: 6 × 2
# char_time hm_time
# <chr>       <dbl>
# 1 05:00           0
# 2 10:57           0
# 3 10:58           0
# 4 16:50           0
# 5 21:59           0
# 6 NA             NA
# Warning message:
#   In .parse_hms(..., order = "HM", quiet = quiet) :
#   Some strings failed to parse, or all strings are NAs

I would expect the transformation to behave like any other function applied inside an ifelse(), such as with this example using stringr::str_remove():


tibble::tibble(
  char_time = c("05:00", "10:57", "10:58", "16:50", "21:59", NA),
  hm_time = ifelse(!is.na(char_time), stringr::str_remove(char_time, ":"), NA))

# # A tibble: 6 × 2
# char_time hm_time
# <chr>     <chr>  
# 1 05:00     0500   
# 2 10:57     1057   
# 3 10:58     1058   
# 4 16:50     1650   
# 5 21:59     2159   
# 6 NA        NA   
vspinu commented 11 months ago

Cannot help with ifelse, as it's a base function which does not understand the period internals, but I have fixed the warnings. NAs now silently propagated as NAs.