pharmaverse / sdtmchecks

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

add unit tests for new diff_reports utility function #295

Open sarabodach opened 6 months ago

sarabodach commented 6 months ago

https://github.com/pharmaverse/sdtmchecks/blob/devel/R/utils.R#L602-L728

rossfarrugia commented 1 month ago

as per Slack message Jim Rothstein is keen to take on this issue once he gets write access.

sarabodach commented 1 month ago

Hi @jimrothstein - thank you for your help!

jimrothstein commented 3 weeks ago

@sarabodach Maybe not smart question, where are sdtm datasets? j from run_all_checks.R

#' # Assuming sdtm datasets are in your global environment

I do see samples AE etc in code for each test, but not AE itself.

sarabodach commented 3 weeks ago

Hi @jimrothstein - that is a good question! We have just used small test datasets in headers to illustrate the data check logic or for testing in existing unit tests. The package itself does not include larger SDTM domains in general.

I haven't tried this myself yet, but there are some pharmaverse packages (such as https://github.com/pharmaverse/pharmaversesdtm) with test SDTM data in case you are interested in seeing other examples that likely have more types of scenarios. We would not want to add this additional package as a dependency though, so I'd advise not to include this package in the actual unit test programs.

I'd suggest starting small with a few AE dataframe examples that are in check headers to see whether the list of data check results A yields expected results when comparing to the list of data check results B. The result of a check may reflect 0 records if the check function passes, so it is not necessary to have all types of data examples for creating unit tests.

jimrothstein commented 3 weeks ago

@sarabodach Greatly appreciate the replies; keeps me from going too far astray.

Thx again.

sarabodach commented 3 weeks ago

Hi @jimrothstein , comments are fine if they are relevant. But they can also be cleaned up later as well in case it's easier for you to move forward with them. Thanks!

jimrothstein commented 3 weeks ago

@sarabodach My goal is unit tests for diff_reports, but I notice that run_check and run_all_checks do not yet have unit tests. They build on each other. (Also forces me to go through code, which is good thing.)

We talked about constructing a list of dummy datasets. So I did that. I then used this dummy list to test run_check, for the one check_ae_aeanoth . Not yet in testthat format.

~~Could you look at code, please; I will rework code and turn it into real testhat code. Thx. (Looks like code is still on my forked repo; should be on branch 295_ . Need to figure this out. Sorry, beginner issues.)~~

Update (Monday, 17 JUNE)