Add requireNamespace for semver package

[bschilder]

This lets users know exactly what they need to do.

Hi @richelbilderbeek,

With this Pull Request I'd would like to make sure users know when and how to install the semver package, which is a Suggest for ormr.

Sure, I've read CONTRIBUTING.md 👍

Cheers,
Brian

[richelbilderbeek]

Hi @bschilder, thanks for the Pull Request!

Could you submit a test to prove the usefulness of it? Now this code would reduce the code coverage without a clear benefit. I'd be happy to learn why this is useful. If you have a reference to the literature, that would be great!

Thanks and cheers, Richel

[bschilder]

I think your current unit tests are actually demonstrating this already!

From my understanding, semver is required to successfully run certain functions (e.g. namely is_python_package_with_version_installed). Therefore that function (and all that depend on it) should fail with an informative error message when semver is not installed. This is ideally checked at the beginning of said function, so that users don't spend time running prior steps that will eventually fail once they get to the semver-dependent step(s).

Also, CRAN and Bioc checks will fail unless you declare Suggests within functions somehow. There's different ways of doing this, but the one I'm most familiar with with using requireNamspace(<pkg>)).

Let me know if I'm misunderstanding the role of semver in ormr, or if you previously satisfied CRAN/Bioc requirements some other way.

[bschilder]

On a related note, I've spent some time modifying and extending the biocthis GHA workflow so that it's more flexible and robust. This includes ensuring that all deps (including Suggests) are installed.

Feel free to make use of it or any of the other scripts in the larger templateR R package template I made:
https://github.com/neurogenomics/templateR

[richelbilderbeek]

From my understanding, semver is required to successfully run certain functions (e.g. namely is_python_package_with_version_installed). Therefore that function (and all that depend on it) should fail with an informative error message when semver is not installed. This is ideally checked at the beginning of said function, so that users don't spend time running prior steps that will eventually fail once they get to the semver-dependent step(s).

Also, CRAN and Bioc checks will fail unless you declare Suggests within functions somehow. There's different ways of doing this, but the one I'm most familiar with with using requireNamspace()).

I completely agree with this reasoning! I feel there are two flaws regardless:

• If one cannot write a test that fails without the new code, that code should not be added. This is just common practice in Test-Driven Development to limit the amount of tests. But GitHub Actions already installs this [edit: this is false, I let semver be installed in the GHA script] [edit: would ormr be on CRAN, however, semver would be installed automatically]
• I miss a citation to the literature, e.g. Advanced R states: 'Inside a package, it’s occasionally useful to check that a package is installed before using it', however, without specifying when 'occasionally' is true.

By accepting this PR I will lose code coverage for sure. I am tempted to just accept to be polite.

We'll discuss this tomorrow :-)