pharmaverse / sdtmchecks

Analysis facing checks for SDTM data
https://pharmaverse.github.io/sdtmchecks/
Apache License 2.0
21 stars 8 forks source link

checks using covid PTs should be parameterized as a list of terms instead of a dataframe #232

Open harriscw opened 1 year ago

harriscw commented 1 year ago

i.e. check_some_check(AE=ae, covid_terms=c("COVID-19","CORONAVIRUS")) instead of check_some_check(AE=ae, covid_terms=dataframe_with_covid_info)

sarabodach commented 1 year ago
Updating befba0b..8f6a19c
Fast-forward
 R/check_ae_aeacn_ds_disctx_covid.R                 |  3 +-
 R/check_ae_aeacnoth_ds_stddisc_covid.R             |  3 +-
 R/check_dv_ae_aedecod_covid.R                      |  3 +-
 .../testthat/test-check_ae_aeacn_ds_disctx_covid.R | 31 +++++++++++++++++++++
 .../test-check_ae_aeacnoth_ds_stddisc_covid.R      | 32 ++++++++++++++++++++++
 tests/testthat/test-check_dv_ae_aedecod_covid.R    | 24 +++++++++++++++-
 6 files changed, 92 insertions(+), 4 deletions(-)

https://github.com/pharmaverse/sdtmchecks/commit/8f6a19ccb4a96a2f836586726da8ceb0c9f84f4d

sarabodach commented 1 year ago

Hi @harriscw - I am a bit concerned about backwards compatibility with this type of change.

harriscw commented 1 year ago

Yeah its something we can think about but for this I don't think backwards compatibility issues should be a blocker. Expecting a vector of terms is way more intuitive IMO than a dataframe with a variable called REFTERM. Internally at Roche we would update our app of course. And we can make the functions fail gently and informatively if a data frame is provided instead of a vector.