search

Gilead-BioStats / gsm

Good Statistical Monitoring R Package
https://gilead-biostats.github.io/gsm/
Apache License 2.0
36 stars 9 forks source link

Standardize gt usage #1657

Closed jonthegeek closed 1 month ago

jonthegeek commented 1 month ago

Overview

Standardize usage of {gt}. Closes #1652.

Test Notes/Sample Code

Notes:

As a step 0, I fixed the dates in sampleSummary, and reconciled the tests with the updates from previous PRs (changing to the sample datasets broke a bunch of things). I wanted tests to pass before I make changes, so I can tell for sure that I didn't break anything.

jonthegeek commented 1 month ago

@jwildfire @lauramaxwell I fixed the broken tests in this to clear the field before I move on to the gt stuff. Can one of you confirm that my changes make sense? A number of tests were looking for specific values, but I'm PRETTY sure those values legitimately changed with the new data.

There's one actual bugfix in there, in Report_Setup.R. It was taking the first date and THEN finding max of that, and I think you meant for it to be the max of the whole column.

Of course now I see a test failing on here so I'll check that before moving on. They SHOULD all pass...

jonthegeek commented 1 month ago

Update: Tests weren't broken, but a number of other things were throwing errors in R CMD check. This version should work. Once the checks are green, I'll implement my feature and make sure I don't break anything.

jonthegeek commented 1 month ago

Issues #1659 and #1660 deal with more complex kable-to-gt cases. This PR does a fair amount of cleaning/refactoring, but the main thing it implements is a standardization of gt() calls (in Report_StudyInfo()).