plotStudy and getPlottingData functions accept parameter testID.

[curadomr]

• Param testID is set to NULL in plotStudy as default.
• Output of getPlottingData provides test result (if testID is NULL, no result object is provided in the plottingData list)

[jdblischak]

This is a great start! I've made some initial suggested changes. You can either accept them here on GitHub, or you can make the changes locally, add/commit the changes, and then push to GitHub. This will automatically update the PR.

[jdblischak]

@curadomr After you address my review comments, the next steps will be adding documentation and tests. For the documentation, start by running devtools::document() (or Ctrl-Shift-D in RStudio). The argument testID should automatically be documented for both functions since they inherit it from @shared-get. You'll also need to manually document the new behavior to return results in the @return section of getPlottingData(). Add and commit the changes to the files in man/. You can read more about how the documentation is generated in the r-pkgs chapter on Object Documentation.

[jdblischak]

@curadomr Now that you applied the suggestions I made, make sure you pull these changes to your local copy before you start updating the documentation:

git pull origin testID

[curadomr]

Suggestions on testID default were implemented/accepted. Furthermore, documentation is modified as suggested (devtools::documentation() and manual entry for getPlottingData result section).

[jdblischak]

Great. The man pages look good. The next step is adding unit tests. Some resources:

1. The section on Unit tests in CONTRIBUTING.md
2. The tinytest vignettes Using tinytest and tinytest by example

I recommend you start by copy-pasting my existing tests of getPlottingData() and modify them to pass a testID and confirm that results are included in the returned object. To keep things organized, you can start a new subsection, e.g. getPlottingData with testID ---. And then you can run the tests by executing the following in the R console:

x <- tinytest::run_test_file("inst/tinytest/testPlot.R")

You can ignore the failing Ubuntu job. It's failing due to some puzzling GitHub Actions behavior where it has applied some but not all of my recent changes to the build steps.

[curadomr]

Just added the changes in testPlot.R for the tinytest. On my side, all tests went well following your suggested command:

x <- tinytest::run_test_file("inst/tinytest/testPlot.R")

[jdblischak]

Tests look good!

Last step is to update the vignettes:

1. Please update the API vignette section on custom plots to pass testID to plotStudy()

2. Since the example plotting functions in the user's guide don't use the results table info (at least not yet), I think for simplicity it makes the most sense to add a note at the end of the section on plots that explains the option to pass the testID to receive the results for plotting

[curadomr]

Please let me know if the modified vignettes suffice for informing the user adequately.

[jdblischak]

Thanks @curadomr! 🎉

To update your local copy and fork, please run the following in the terminal:

git checkout main
git pull upstream main
git push origin main