Bug: `cal_min_max_date()` fails when `time_variable` has `NA` values

[ramiromagno]

@ShiyuC: This is my narrowing down of the problem you shared with me. Your example included a call to oak_cal_ref_dates() but it is actually (at least) cal_min_max_date() that is having problems. I'm assigning this issue to you as you are the bearer of the original bug but feel free to delegate on me if Ramm and Mohsin feel like having me taking over this function's code.

@rammprasad, @muzzama-1990: The function cal_min_max_date() was written by you, so could you take a look? I'm happy to take over but after looking at the code I would overhaul it entirely...

Reproducible example

library(sdtm.oak)
  
tibble::tibble(
  patient_number = "001",
  EX_ST_DT = "25-04-2022",
  EX_ST_TM = "15:10") |> 
  cal_min_max_date(
    date_variable = "EX_ST_DT",
    time_variable = "EX_ST_TM",
    date_format = "d-m-y",
    time_format = "H:M"
  )
#> # A tibble: 1 × 2
#>   patient_number datetime        
#>   <chr>          <chr>           
#> 1 001            2022-04-25T15:10

create_iso8601(EX_ST_DT = "25-04-2022", EX_ST_TM = "15:10", .format = c("d-m-y", "H:M"))
#> [1] "2022-04-25T15:10"

# `cal_min_max_date()` fails to derive "2022-04-25" but it ought not.
tibble::tibble(
  patient_number = "001",
  EX_ST_DT = "25-04-2022",
  EX_ST_TM = NA_character_) |> 
  cal_min_max_date(
    date_variable = "EX_ST_DT",
    time_variable = "EX_ST_TM",
    date_format = "d-m-y",
    time_format = "H:M"
  )
#> # A tibble: 0 × 2
#> # ℹ 2 variables: patient_number <chr>, datetime <chr>

# `create_iso8601()` works as expected illustrating that the problem comes
# from `cal_min_max_date()`, not from `create_iso8601()`.
create_iso8601(EX_ST_DT = "25-04-2022", EX_ST_TM = NA_character_, .format = c("d-m-y", "H:M"))
#> [1] "2022-04-25"

[ramiromagno]

I think we can simplify tremendously by replacing cal_min_max_date() / oak_cal_ref_dates() with a simple function derive_ref_date() that is more in line with the other existing assign_*() and hardcode_*() functions, takes advantage of assign_datetime() and is more flexible.

Let me know if you would like a PR!

Here's the PoC:

library(sdtm.oak)

derive_ref_date <- function(raw_dat,
                            ref,
                            raw_var,
                            tgt_var,
                            raw_fmt,
                            raw_unk = c("UN", "UNK"),
                            ptid = "patient_number") {
  
  ref <- match.arg(ref, choices = c("NOCB", "LOCF", "ST", "EN"))
  slice <- if (ref %in% c("ST", "NOCB")) dplyr::slice_min else dplyr::slice_max
  na_rm <- ref %in% c("NOCB", "LOCF")
  
  assign_datetime(
    raw_dat = raw_dat,
    tgt_var = tgt_var,
    raw_var = raw_var,
    raw_fmt = raw_fmt
  ) |>
    dplyr::group_by(!!!rlang::syms(ptid)) |>
    slice(order_by = .data[[tgt_var]], n = 1L, na_rm = na_rm) |>
    dplyr::ungroup()
  
}

my_data <-
  tibble::tibble(
    oak_id = 1:5,
    raw_source = "MD1",
    patient_number = c("001", "001", "002", "002", "002"),
    DSDTCOL = c("25-04-2022", "14-01-2023", NA_character_, "01-12-2000", "01-06-2001"),
    DSTMCOL = c(NA_character_, "10:05", NA_character_, "16:00", "19:00"))

my_data
#> # A tibble: 5 × 5
#>   oak_id raw_source patient_number DSDTCOL    DSTMCOL
#>    <int> <chr>      <chr>          <chr>      <chr>  
#> 1      1 MD1        001            25-04-2022 <NA>   
#> 2      2 MD1        001            14-01-2023 10:05  
#> 3      3 MD1        002            <NA>       <NA>   
#> 4      4 MD1        002            01-12-2000 16:00  
#> 5      5 MD1        002            01-06-2001 19:00

derive_ref_date(
  raw_dat = my_data,
  ref = "NOCB",
  tgt_var = "RFPENDTC",
  raw_var = c("DSDTCOL", "DSTMCOL"),
  raw_fmt = c("d-m-y", "H:M")
)
#> # A tibble: 2 × 4
#>   oak_id raw_source patient_number RFPENDTC        
#>    <int> <chr>      <chr>          <iso8601>       
#> 1      1 MD1        001            2022-04-25      
#> 2      4 MD1        002            2000-12-01T16:00

derive_ref_date(
  raw_dat = my_data,
  ref = "LOCF",
  tgt_var = "RFPENDTC",
  raw_var = c("DSDTCOL", "DSTMCOL"),
  raw_fmt = c("d-m-y", "H:M")
)
#> # A tibble: 2 × 4
#>   oak_id raw_source patient_number RFPENDTC        
#>    <int> <chr>      <chr>          <iso8601>       
#> 1      2 MD1        001            2023-01-14T10:05
#> 2      5 MD1        002            2001-06-01T19:00

[muzzama-1990]

Hi @ramiromagno I am sorry for the delay in response. If this can be simplified and will be efficient then I am ok that this can be modified as you suggested.

[zhengzheli]

@ramiromagno does this mean the ref_date_conf_df used for RFSTDTC, RFXSTDTC, etc. will no longer be needed? Can it handle finding a minimum or maximum among several date variables?