search

pharmaverse / admiral

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

program function to derive LSTALVDT #94

Closed kabis-ops closed 3 years ago

bundfussr commented 3 years ago

I had a look at the standard DAP M3. There are other derivations which require a similar algorithm like LSTALVDT. For example the ADJ, ADJME, ADJAE parameters in ADEX, the time to event parameters in ADTTE, and the DTHDOM, DTHCAUS, and ADTHAUT. In principle it would be possible to implement a general function which covers all these derivations. However, it is not clear in which direction admiral should go. I think the concerns Robin raised regarding derive_merged_vars() are related. I have put it on the agenda for next Monday.

bundfussr commented 3 years ago

Hi @kabis-ops , hi @thomas-neitmann , could you have a look at the derive_extreme_date_var() function in the 94_LSTALVDT branch? Is it general enough or too general?

thomas-neitmann commented 3 years ago

@bundfussr Please open a pull request and mark it as draft. That makes the review much easier.

bundfussr commented 3 years ago

@thomas-neitmann , done, see https://github.com/Roche-GSK/admiral/pull/184

kabis-ops commented 3 years ago

@bundfussr - i think i would have preferred a function focusing only on lstalvdt only. 

derive_var_lstalvdt(dataset, source)

Keeping it simple like for dthdom/dthcaus. But open to discuss :)

@thomas-neitmann: what do you think?

bundfussr commented 3 years ago

@kabis-ops , @thomas-neitmann , what about imputation? At the moment I assume that all DTC dates are complete. Should we offer a parameter for imputation?

kabis-ops commented 3 years ago

Hi @bundfussr, yes, we should include a param for the date imputation.