Bug: The result of derive_param_tte() depends on the sort order of the input

[bundfussr]

What happened?

The result of derive_param_tte() depends on the sort order of the input. See example below.

An order argument should be added to event_source() and censor_source(). It should be defaulted to NULL. It expects a list of variables, which are used in addition to date for sorting the input data.

The check_type argument should be added to derive_param_tte() and defaulted to "warning". The uniqueness of the selected records should be checked by calling signal_duplicate_records().

Session Information

No response

Reproducible Example

adsl <- tibble::tribble(
  ~USUBJID, ~TRTSDT,           ~TRTEDT,           ~EOSDT,
  "01",     ymd("2020-12-06"), ymd("2021-03-02"), ymd("2021-03-06"),
  "02",     ymd("2021-01-16"), ymd("2021-01-20"), ymd("2021-02-03")
) %>%
  mutate(STUDYID = "AB42")

ae <- tibble::tribble(
  ~USUBJID, ~AESTDTC,     ~AESEQ, ~AEDECOD,
  "01",     "2021-01-03",      1, "Flu",
  "01",     "2021-03-04",      2, "Cough",
  "01",     "2021-01-03",      3, "Flu"
) %>%
  mutate(
    STUDYID = "AB42",
    AESTDT = ymd(AESTDTC)
  )

ttae <- event_source(
  dataset_name = "ae",
  date = AESTDT,
  set_values_to = exprs(
    EVENTDESC = "AE",
    SRCDOM = "AE",
    SRCVAR = "AESTDTC",
    SRCSEQ = AESEQ
  )
)

eot <- censor_source(
  dataset_name = "adsl",
  date = pmin(TRTEDT + days(10), EOSDT),
  censor = 1,
  set_values_to = exprs(
    EVENTDESC = "END OF TRT",
    SRCDOM = "ADSL",
    SRCVAR = "TRTEDT"
  )
)

derive_param_tte(
  dataset_adsl = adsl,
  start_date = TRTSDT,
  event_conditions = list(ttae),
  censor_conditions = list(eot),
  source_datasets = list(adsl = adsl, ae = arrange(ae, AESEQ)),
  set_values_to = exprs(
    PARAMCD = "TTAE"
  )
)

produces

# A tibble: 2 × 10
  USUBJID STUDYID EVENTDESC  SRCDOM SRCVAR  SRCSEQ  CNSR ADT        STARTDT    PARAMCD
  <chr>   <chr>   <chr>      <chr>  <chr>    <dbl> <int> <date>     <date>     <chr>  
1 01      AB42    AE         AE     AESTDTC      1     0 2021-01-03 2020-12-06 TTAE   
2 02      AB42    END OF TRT ADSL   TRTEDT      NA     1 2021-01-30 2021-01-16 TTAE   

but

derive_param_tte(
  dataset_adsl = adsl,
  start_date = TRTSDT,
  event_conditions = list(ttae),
  censor_conditions = list(eot),
  source_datasets = list(adsl = adsl, ae = arrange(ae, desc(AESEQ))),
  set_values_to = exprs(
    PARAMCD = "TTAE"
  )
)

produces

# A tibble: 2 × 10
  USUBJID STUDYID EVENTDESC  SRCDOM SRCVAR  SRCSEQ  CNSR ADT        STARTDT    PARAMCD
  <chr>   <chr>   <chr>      <chr>  <chr>    <dbl> <int> <date>     <date>     <chr>  
1 01      AB42    AE         AE     AESTDTC      3     0 2021-01-03 2020-12-06 TTAE   
2 02      AB42    END OF TRT ADSL   TRTEDT      NA     1 2021-01-30 2021-01-16 TTAE   

[bms63]

Is this a bug we need to push a patch out for or something we can wait on till next release?

[rossfarrugia]

I'd say next release as its not a bug as such (the function does what it says) - its more just the lack of a useful feature

[bms63]

Should I tag the team and community team to see if anyone is interested or do we feel like this is a bit more complicated?

[bundfussr]

I think it is something in between a bug and a feature. Maybe we should call it a gap because for the scenario of more than one record per date the expected behavior of the function is not described.

I think it is OK to tag the team and community team but it shouldn't be labelled as "good first issue". It could be a good issue for team members who have some time and are interested in getting familiar with one of the more complex admiral functions.

[bms63]

@pharmaverse/admiral and @pharmaverse/admiral_comm this is a good one to tackle, but it is not a "good first issue". You will get into the weeds a bit, but great way to learn more about admiral!!

[ProfessorP-beep]

@bms63 @bundfussr @rossfarrugia I have some free time and can take a look. When is our next release date again?

[rossfarrugia]

thanks @ProfessorP-beep - it's early Dec: see https://pharmaverse.github.io/admiral/#release-schedule

[rossfarrugia]

Ha sorry @manciniedoardo - i quoted from the site, but think I remember you and Ben saying it'll push back to Jan

[bms63]

Ha sorry @manciniedoardo - i quoted from the site, but think I remember you and Ben saying it'll push back to Jan

at the rate we are going we might just do a release in early December. Work has really chilled out! Unless you want to lead some huge refactor @rossfarrugia ??

[rossfarrugia]

@ProfessorP-beep will this one be ready for the next release? If you were able to get the updates in main in the next month or so then I could do some testing on the Roche side as we need to make updates to our template code when this update comes.

[ProfessorP-beep]

@ProfessorP-beep will this one be ready for the next release? If you were able to get the updates in main in the next month or so then I could do some testing on the Roche side as we need to make updates to our template code when this update comes.

@rossfarrugia Sorry, things got very chaotic at work and I had to shift focus. Give me a week and I can update you on if I can get it done by the next release.

[ProfessorP-beep]

Should have a lot of time to work on this the rest of the week and aiming to get a push out by Friday / Saturday.