pharmaverse / admiral

ADaM in R Asset Library
https://pharmaverse.github.io/admiral
Apache License 2.0
222 stars 62 forks source link

Should we enhance `derive_var_ontrtfl()`? #726

Closed bundfussr closed 2 years ago

bundfussr commented 2 years ago

Testers feedback: 1) >Ontrtfl function needs to be modified in order to account for scenarios which often happen in Oncology trials (ONTRTFL should contain a parameter to allow for inclusion of a subsequent therapy date). please include a parameter to add a subsequent therapy as a treatment period "breaker". I would rename filter_pre_timepoints to something like "filter_out_pretpt"? Just to avoid an impression we are trying to SELECT pre-treatment timepoints; That's just me being picky but filtering typically associates with a "positive" choice. Here we filter them away. Nothing serious. I’ll be happy if it stays as it is.

2) >a). Is it possible to add conditions for "ref_end_window"? Like oncology trials, first date of subsequent anti-cancer is also considered.

b). When ASTDT < TRTSDT and AENDT = TRTSDT, ONTRTFL will not be flagged as Y?

3) >The function derive_var_ontrtfl doesn’t work well if the start_date is not generated from function derive_vars_dt()

4) >Higher level comment: As such the derive_var_ontrtfl() can create the perhaps most used version of on-treatment periods: one long period from one date to the other. If it could reflect on-treatment periods more flexible the 'filter_pre_timepoint' might have need for extension. So the function can cover drug-holidays (for treatment periods that do not cover if patient did not take drug), also cross-over-trials seem to be more complex in on-treatment periods than the functions can cover. But do not have an exact example to test on, hence this is only a overall input comment.

rossfarrugia commented 2 years ago

1 & 2a)Couldn't this be achieved using ref_end_date argument @bundfussr? Maybe you'd just need a step before to take minimum of NACTDT and TRTEDT as a temp variable and feed this in. Or is it worth allowing multiple variables to be included here in the argument?

2b)Yes that's my understanding. If an event started pre-treatment, then it doesn't get flagged as on-treatment, even if ended after treatment started. Unless the user uses the span_period="Y" option.

3)We would need more detail here. It should work for any DT or DTM variable.

4)We haven't yet come to look into crossover trials for admiral or study-specific periods. This will take a lot more effort, once we have enough of a stable foundation covering all the common needs for parallel trials first.

bundfussr commented 2 years ago

@rossfarrugia , yes, for 1 & 2a) you could create a temporary variable in a pre-processing step (you would need to take ref_end_window into account in the pre-processing step). I would not accept multiple variables for ref_end_date because then it is not clear to which one ref_end_window applies. We could add a separate parameter like max_ref_end_date = NACTDT. But most likely the temporary variable is more readable.

Regarding 4) I think the function should not handle treatment interruptions because it requires a completely different algorithm. You would need to join with EX or ADEX observations or use the period variables.

rossfarrugia commented 2 years ago

@bundfussr i agree - i'll close this issue as i don't deem any updates needed to the function for these comments.