Enhancement/run app via package installation

[m-kolomanski]

Issue

Closes #122

Description

This PR supersedes #123. The old PR was done on a very outdated branch, I have decided to implement changes to the most up-to-date version of the app instead of dealing with conflicts.

This changes brings ability to run the app after installing the package - without cloning the repository.

Changes:

• update `aNCA::run_app()
  • add ... to the signature to be able to pass arguments to shiny::runApp() such as host, port, etc
  • remove all import comments due to their redundant nature: the app should require all imports that are listed in the packages NAMESPACE file
  • call require() insider the function to load all required libraries
• fixes:
  • export some utility functions that are used by the app
  • fix function namespacing in test files

Definition of Done

• user can install the package from github via pak::pak("pharmaverse/aNCA")
• user can run the app via aNCA::run_app()
• README contains information on how to install & run the app
• Package version is bumped

How to test

Single command to test the app via package installation (requires docker):

docker run --rm -it -p 3838:3838 rocker/r-ver:latest bash -c "apt update && apt install libcurl4-openssl-dev && Rscript -e \"install.packages('pak'); pak::pak('pharmaverse/aNCA@enhancement/run-app-via-package-installation2')\" && Rscript -e \"aNCA::run_app(host='0.0.0.0',port=3838)\""

Alternatively, install the package in your environment via pak from this branch:

pak::pak('pharmaverse/aNCA@enhancement/run-app-via-package-installation2')

If you are installing the package without docker, please use some sort of virtual environment management system (like renv) OR make sure to start with a clean slate and remove the current installation that is present in your global library by running remove.packages('aNCA').

Contributor checklist

• Code passes lintr checks
• Code passes all unit tests

Notes to reviewer

Please perform the functional testing rigorously. I have performed a simple analysis and clicked through all the options, but there is a high probability that I have missed something. There might be a chance that some packages were not properly put into the NAMESPACE, and due to that are not loaded in the app.

In addition, if you notice any packages that are loaded but should not due to being outdated / deprecated / superseded / changed for something else and mistakenly not removed from the NAMESPACE file, please do let me know.

[js3110]

Hi @m-kolomanski , I went through and checked all the features, and everything seems to be working the same as when I was going through the cloned version. So I am happy to approve!

[Gero1999]

Thanks for doing this! I did the next review, let me know if anything else is needed from my side:

1. Local installation by cloning works fine
2. Installation with pak 0.8.0.1 also works
3. Main apps functionalities are not affected. Seems all packages are being imported.

I only made some comments to the README with potential improvements. But I guess the README won't create big crashes if we merge!

[Gotfrid]

I tested it in docker according to the instruction and everything seems to work fine. Awesome work, and thank you so much for finding time to bring the stale PR back to life! 🎉

I have only one general thought that I also had when originally working on this PR: it feels sketchy to include a package into DESCRIPTION but not to have a single @import or @importFrom instruction in the code. I mean, it makes sense to all of us now, but if a new contributor joins the project it might not make sense to them. Personally, I'd be surprised and would want to "clean up" the DESCRIPTION file 😄

Maybe a line of caution in the README would make sense, or maybe you think this worry is superficial?

[Gero1999]

I tested it in docker according to the instruction and everything seems to work fine. Awesome work, and thank you so much for finding time to bring the stale PR back to life! 🎉

I have only one general thought that I also had when originally working on this PR: it feels sketchy to include a package into DESCRIPTION but not to have a single @import or @importFrom instruction in the code. I mean, it makes sense to all of us now, but if a new contributor joins the project it might not make sense to them. Personally, I'd be surprised and would want to "clean up" the DESCRIPTION file 😄

Maybe a line of caution in the README would make sense, or maybe you think this worry is superficial?

Hey, instead of in the README do you think would it make sense to include this line of caution in the CONTRIBUTING.md and/or as a comment in the DESCRIPTION file?

[Gotfrid]

@Gero1999 I'm not sure if comments are allowed in the DESCRIPTION file, but CONTRIBUTING.md is a great idea! 💡

[m-kolomanski]

Hey @Gotfrid , @Gero1999,

thank you for the feedback! I have had the same thought, but decided to leave it as-is, since I believe that before publishing the package on CRAN we should go through all dependencies and cut down on the number. We have been adding dependencies, but I do not think we have been removing any - so there is a change we are requiring unused packages. Before publishing we should remove those unused dependencies, plus see if we can replace any with either base functions or more prominent packages.

Nevertheless, this entry to the CONTRIBUTING file will always be useful considering the structure of the package, so I have added it.