Add minimum required version of dependency packages to DESCRIPTION file

[thomas-neitmann]

Background Information

Currently there is no minimum required version for {admiral.test} indicated in our DESCRIPTION file. However, the latest release only works with version 0.2.0 of {admiral.test}. If you already have {admiral.test} installed and install the latest version of {admiral} then {admiral.test} won't get updated automatically. This will lead to errors when running examples.

I think this might actually warrant a patch release, i.e. v0.7.1 of {admiral}. What do you think @pharmaverse/admiral?

Definition of Done

DESCRIPTION fiel contains admiral.test (>= 0.2.0) line.

[bms63]

I think a mini-release makes sense. The admiral_ update will haunt us for a while...

[thomas-neitmann]

@bms63 Also something we should add to our release check list.

[thomas-neitmann]

Alright, I'll go ahead make this change and create a new release.

[thomas-neitmann]

Actually I just discoverd that {admiral.test} is not automatically installed when running install.packages("admiral") because it's listed in "Suggests" rather than "Imports" in the DESCRIPTION file. I think the rationale was that {admiral.test} is not really required to use {admiral} but only to run the examples. However, being able to run the examples does seem like quite an important piece. So we might want to change that. Any thoughts @bms63 @bundfussr?

[bundfussr]

Moving admiral.test to "Imports" is fine with me.

[bms63]

I made this issue more generic as we need this or tidyr, rlang and diffdf now. Others?

[esimms999-gsk]

Summary of discussion between Eric and Thomas on Slack:

Eric: Any hints as to how I determine the minimum version of a dependency, e.g. how would I figure out the minimum version of stringr which needs to be available for admiral to run without problems? Looking at NAMESPACE, I can see all the importFrom statements. Looking at just the tidyr ones: importFrom(tidyr,drop_na) importFrom(tidyr,nest) importFrom(tidyr,pivot_longer) importFrom(tidyr,pivot_wider) importFrom(tidyr,unnest) I know that pivot_longer and pivot_wider were introduced at some point in the last few years and I could track down what version of tidyr had these functions available, But what about all the others, such as nest, unnest, etc.? Is this just a slow, manual task to figure out minimum versions for dependencies or is there a better way to do this that you know of?

Thomas: The {tidyr} example certainly is the most easy one. As you've said, these two function got introduced in a certain version so that's the minimum version we require. Next you could look at the minimum version dependency of that exact {tidyr} version. I know both {admiral} and {tidyr} depend on {rlang}. So if our required version of {tidyr} requires {rlang} v0.9.0 then that's also our minimum required version unless we use some more recent features. (edited) Apart from that a very conservative approach would be to list the version we have in our renv.lock file as minimum requirements. I do feel that this would be overly restrictive, though. However, it's a place to start.

[esimms999-gsk]

[esimms999-gsk]

From DESCRIPTION file:

[thomas-neitmann]

Of note, please focus on the packages listed on "Imports". These are the "actual" hard dependencies that {admiral} does not work without. "Suggests" are mainly packages we use during development but which have no impact on the package's functionality from a user's perspective.

[esimms999-gsk]

I do not know if this is a problem or not: NAMESPACE and renv.lock have package "utils", but it is not mentioned in DESCRIPTION.

[thomas-neitmann]

That's a "core" package that ships with the default installation of R so no need of it being explicitly declared as a dependency in the DESCRIPTION file.

[esimms999-gsk]

Things did not go as smoothly as I wanted while working on this. It came down to a lot of guessing, trial and error ... hopefully due to my lack of knowledge about packages; maybe an automated solution is available and I am just unaware of it.

R v3.6.3 (our minimum permitted) was released 2020-02-29, and I assume a user or developer installed it on that date. If they did so, the packages they would have loaded would have been whatever was available on that date. For the packages of interest (importFrom in NAMESPACE), the packages available on that date (making use of groundhog::toc) are in the column "groundhog: latest version as of 2020-02-29". Out of the 11 packages, our current renv.lock setting for 7 of them is the same version as would have been available on 2020-02-29, e.g. our renv.lock for assertthat is version 0.2.1, which was the version available on 2020-02-29. For these 7, our minimum package version is a good choice. For the remaining 4 (dplyr, lifecycle, magrittr, rlang), perhaps our minimum version is more recent than necessary. I think we can get away with dplyr 0.8.4 (released 2020-01-31, so before R 3.6.3), lifecycle 1.0.0 (released 2021-02-15), magrittr 2.0.1 (released 2020-11-17) and rlang 1.0.0 (released 202201-26). This needs to tested for both user and developer role. I thought I could get away with earlier versions of magrittr and rlang, but pulling down the repo as a developer seems to have additional need for more recent versions, e.g. the styler package is not happy with magrittr 1.5. I have made a branch named 1204_eric_minimum_version_of_packages; the only change is the version number in renv.lock for the 4 packages. To install as a user:

if (!requireNamespace("remotes", quietly = TRUE)) { install.packages("remotes") } remotes::install_github("pharmaverse/admiral.test", ref = "devel") # This is a required dependency of {admiral} remotes::install_github("pharmaverse/admiral", ref = "1204_eric_minimum_version_of_packages", force = TRUE)

The results of the automatic CMD Check on install looks good. As a developer, I pulled down the repo, jumped to my branch and did Test and Check, which look good. If there are other tests which should be done, please let me know. I am very interested in any comments on this.

[thomas-neitmann]

Thanks a lot @esimms999-gsk!

This needs to tested for both user and developer role. I thought I could get away with earlier versions of magrittr and rlang, but pulling down the repo as a developer seems to have additional need for more recent versions, e.g. the styler package is not happy with magrittr 1.5.

Minimum version for developers is out of scope. As a develoepr you would just run renv::restore() to get the same packages installed as every other user. For users only the packages listed under "Imports" are of interest plus {admiral.test} for some of the examples.

[thomas-neitmann]

I jsut checked out branch 1204_eric_minimum_version_of_packages, run renv::restore() and then devtools::check(). All examples and tests pass so this set of package version does seem to be a suitable set of minimum versions.

[thomas-neitmann]

Just check with {magrittr} v1.5 and devtools::check() again runs smoothly.

[esimms999-gsk]

@thomas-neitmann My understanding is that you are testing this out as a developer, correct? And magrittr v1.5 is ok? I'll try this out.

How about when you test it as a user?

[thomas-neitmann]

No I'm testing from a user's perspective. From a dev's perspective {magrittr} v1.5 wouldn't suffice because we also use {styler} in development but that doesn't concern a user.

[thomas-neitmann]

If devtools::check() works with an installed version we can be almost 100% sure that it wouldn't ever cause any issues for a user who has that particular version installed.

[esimms999-gsk]

Code used to test minimum packages:

install.packages("remotes") install.packages("devtools")

devtools::install_version("assertthat", version = "0.2.1", upgrade = "never", repos = "http://cran.us.r-project.org") devtools::install_version("dplyr", version = "1.0.0", upgrade = "never", repos = "http://cran.us.r-project.org") devtools::install_version("hms", version = "0.5.3", upgrade = "never", repos = "http://cran.us.r-project.org") devtools::install_version("lifecycle", version = "1.0.0", upgrade = "never", repos = "http://cran.us.r-project.org") devtools::install_version("lubridate", version = "1.7.4", upgrade = "never", repos = "http://cran.us.r-project.org") devtools::install_version("magrittr", version = "1.5", upgrade = "never", repos = "http://cran.us.r-project.org") devtools::install_version("purrr", version = "0.3.3", upgrade = "never", repos = "http://cran.us.r-project.org") devtools::install_version("readxl", version = "1.4.0", upgrade = "never", repos = "http://cran.us.r-project.org") devtools::install_version("rlang", version = "1.0.2", upgrade = "never", repos = "http://cran.us.r-project.org") devtools::install_version("stringr", version = "1.4.0", upgrade = "never", repos = "http://cran.us.r-project.org") devtools::install_version("tidyr", version = "1.0.2", upgrade = "never", repos = "http://cran.us.r-project.org") devtools::install_version("tidyselect", version = "1.1.0", upgrade = "never", repos = "http://cran.us.r-project.org")

remotes::install_github("pharmaverse/admiral.test", ref = "devel") # This is a required dependency of {admiral}

remotes::install_github("pharmaverse/admiral", ref = "1204_minimum_package_versions@devel")

This was originally run on R v3.6.3 with dplyr 0.8.5 and tidyselect 1.0.0. Error on dplyr 0.8.5 that 1.0.0 was needed; change made. Re-run and error on tidyselect 1.0.0 that 1.1.0 was needed; change made. Re-run and no error. Re-run on R v4.2.1 and no error.