Closes #106 Updated README.md with correct instructions for adding new SDTM datasets

[fshanlee]

and updating existing SDTM datasets.

Thank you for your Pull Request!

Please check off each taskbox as an acknowledgment that you completed the task or check off that it is not relevant to your Pull Request.

• [x] Place Closes # into the beginning of your Pull Request Title (Use Edit button in top-right if you need to update)
• [x] Code is formatted according to the tidyverse style guide. Run styler::style_file() to style R and Rmd files
• [x] Update to all relevant roxygen headers and examples
• [x] Run devtools::document() so all .Rd files in the man folder and the NAMESPACE file in the project root are updated appropriately
• [x] Update NEWS.md if the changes pertain to a user-facing function (i.e. it has an @export tag) or documentation aimed at users (rather than developers)
• [x] Run R CMD check locally and address all errors and warnings - devtools::check()
• [x] Link the issue in the Development Section on the right hand side.
• [x] Address all merge conflicts and resolve appropriately
• [x] Pat yourself on the back for a job well done! Much love to your accomplishment!

[fshanlee]

Can you add a NOTE: around developers will need to install packages not currently availabe in the lock file, e.g. metatools. Can you look through the scripts and see if there are others.

Just to clarify: If a developer needs to use a package in order to create/update their SDTM data, and if that package has not been specified in the renv lock file, then they will need to install that package. However, if their code does not require a package, then there is no requirement to install it--i.e. they don't need to install {metatools} unless their program uses {metatools} etc. So the Read Me instructions should not specify a list of packages that developers will need to install; instead, we should instruct developers to install any packages that they require in addition to those specified in the lock file--is that correct?

[bms63]

Can you add a NOTE: around developers will need to install packages not currently availabe in the lock file, e.g. metatools. Can you look through the scripts and see if there are others.

Just to clarify: If a developer needs to use a package in order to create/update their SDTM data, and if that package has not been specified in the renv lock file, then they will need to install that package. However, if their code does not require a package, then there is no requirement to install it--i.e. they don't need to install {metatools} unless their program uses {metatools} etc. So the Read Me instructions should not specify a list of packages that developers will need to install; instead, we should instruct developers to install any packages that they require in addition to those specified in the lock file--is that correct?

I might be cautious around allowing developers to install any package they desire to develop the test data. Currently, the only packages not in the lock file are metatools and ggplot2. I haven't done a full review of the code. Could you please look through? There was labelled but I am hoping to replace that with metatools. So providing the list is still helpful and a NOTE on discussing with core-team around using other packages before implementing in their code.

I just don't want to deal with a package that is kicked off CRAN and not accessible down the road.

[fshanlee]

@pharmaverse/admiral , in {admiral.test}, many of the programs in the dev/ folder load {admiral} and {admiral.test}, i.e.:

library(admiral)
library(admiral.test)

Is it correct to load {admiral.test} at the start of a program that generates data that will become part of {admiral.test}? If there are dependencies, for example, the output from tr.R is the input for tu.R, and if both programs are updated before the package is built, then is there a risk that tu.R will read an out-of-date version of the output from tr.R, i.e. the admiral_tr dataset that was created when the package was last built, rather than the admiral_tr dataset that is created by running the updated tr.R?

[fshanlee]

Hi @bms63 , pc.R loads {plyr} in addition to {dplyr}. The {plyr} package is not specified in renv.lock, but I have not yet mentioned it in README.md. Do you know anything about {plyr}? Should we mention it in addition to {metatools} and {ggplot2}?

library(plyr)

[bms63]

Hi @bms63 , pc.R loads {plyr} in addition to {dplyr}. The {plyr} package is not specified in renv.lock, but I have not yet mentioned it in README.md. Do you know anything about {plyr}? Should we mention it in addition to {metatools} and {ggplot2}?

library(plyr)

I don't think we need plyr. I think that was an early version of dplyr

[bms63]

@millerg23 @bundfussr @rossfarrugia Shan is updating the instructions. Can we get you to take a look at these as well as you three have been the most active on this package.

[rossfarrugia]

@millerg23 @bundfussr @rossfarrugia Shan is updating the instructions. Can we get you to take a look at these as well as you three have been the most active on this package.

I'm out of touch on this one sorry Ben - would take me some time to dig back into the details so hopefully one of the others could take a look

[bms63]

Thanks Shan!