use skip_if_not_installed() over requireNamespace branch

NorskRegnesentral / shapr

Explaining the output of machine learning models with more accurately estimated Shapley values

https://norskregnesentral.github.io/shapr/

Other

138 stars 32 forks source link

use skip_if_not_installed() over requireNamespace branch #320

Closed MichaelChirico closed 1 year ago

MichaelChirico commented 1 year ago

This is preferable for a variety of reasons, e.g.

Save an extra indentation
Testthat report output is clearer by indicating that certain tests were skipped if MASS is missing
skip_if_not_installed() is more readable than if (!requireNamespace(., quietly = TRUE))

martinju commented 1 year ago

Thanks for this, I agree with your suggestion. As written in the top of the readme, the package is undergoing big structural changes. In particular, all tests are rewritten, such that it does not make sense to merge this now. So far these tests only use package available in all R installations, but I will take your advice when moving outside of that. Thanks again. Closing this now.