Fix #276 - Create pkg_risk wrapper func

[parmsam-pfizer]

Based on issue #276.

[codecov-commenter]

Codecov Report

Merging #305 (17fe079) into master (3060d6a) will increase coverage by 2.32%. The diff coverage is 87.80%.

:exclamation: Current head 17fe079 differs from pull request most recent head 8c84dbd. Consider uploading reports for the commit 8c84dbd to get more accurate results

@@            Coverage Diff             @@
##           master     #305      +/-   ##
==========================================
+ Coverage   59.23%   61.55%   +2.32%     
==========================================
  Files          64       67       +3     
  Lines         991     1030      +39     
==========================================
+ Hits          587      634      +47     
+ Misses        404      396       -8     

Files Changed	Coverage Δ
R/pkg_ref_cache.R	9.09% <ø> (ø)
R/pkg_ref_cache_NEWS_urls.R	18.18% <ø> (ø)
R/pkg_ref_cache_archive_release_date.R	0.00% <ø> (ø)
R/pkg_ref_cache_bug_reports.R	64.70% <ø> (ø)
R/pkg_ref_cache_bug_reports_host.R	100.00% <ø> (ø)
R/pkg_ref_cache_bug_reports_url.R	55.55% <ø> (ø)
R/pkg_ref_cache_covr_coverage.R	100.00% <ø> (ø)
R/pkg_ref_cache_description.R	100.00% <ø> (ø)
R/pkg_ref_cache_downloads.R	100.00% <ø> (ø)
R/pkg_ref_cache_expr_coverage.R	0.00% <ø> (ø)
... and 19 more

... and 1 file with indirect coverage changes

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

[parmsam-pfizer]

@emilliman5, @elimillera Please let me know if there are any additional checks, messages, or arguments we want in the wrapper function. Right now, it's calling the core function pattern: pkg_ref() to tibble coercion to pkg_assess() to pkg_score(). There's a simple check for a character vector of package names (the primary arg). Also, there's a message when the default pkg_install source is used. I don't expect it to be ready yet, but hope this will spark more discussion.

[emilliman5]

Messaging and parameters are a good improvement. I think the default source should be "pkg_cran_remote" this will reduce the number of errors when someone supplies a long-ish vector of packages where some are installed and some not. Longer term, we will need some logic (messaging or heuristics) to handle the case of different sources. A few options:

1) Error and force user to only provide packages that are available from same source (not nescessarily the most user friendly) 2) remove packages that are not available from given source and provide message 3) instaniate pkg_ref for packages with preferred source (based on argument value). for any packages not available fall back to another source e.g. pkg_risk(c("dplyr","spongebob")) dplyr is installed so use "pkg_install" but spongebob is not so try CRAN, then Bioc then github. 4) something even better that i have not thought of

[parmsam-pfizer]

Thanks for the suggestions, @emilliman5. Updated branch to reflect our latest discussion in this PR. Updated documentation and created pkg_risk_cran() as an alternative for folks who want to ensure they point to "pkg_cran_remote". Hopefully, this will also inform users about the difference source types.

I like the nature of the pkg_cran_remote class that retains a row for the requested package that aren't found with a pkg_missing response in the pkg_ref column. Example of this: pkg_risk_cran(c("riskmetric", "dplyr", "abc", "doesntexist"))

I'd like to replicate that for the pkg_install source type. Right now, it errors out when the "pkg_install" source is used but the package isn't found. What are your thoughts? pkg_risk(c("riskmetric", "dplyr", "doesntexist"), source = "pkg_install")

[parmsam-pfizer]

Hi, @emilliman5! I'd love to know your thoughts on the changes when you get a chance. Thanks!

[emilliman5]

I really like pkg_risk_cran! A few thoughts:

1) Can we make one function to handle all sources (like this pkg_risk(c("riskmetric", "dplyr", "doesntexist"), source = "pkg_install")) 2) Doug had written a simple heuristic to handle the error you describe above in an early version of pkg_ref I think may be a good starting point 3) Lets investigate allowing more intuitive options for the source parameter. e.g. allow "cran" for "pkg_cran_remote" etc.

Lastly, My apologies for radio silence. I think I have finally freed up some bandwidth to dedicate to riskmetric once again.

[parmsam-pfizer]

Sorry for the delay. Made a few updates to handle the error when the "pkg_install" source is used but the package isn't found. It should now retain a row for the requested package that wasn't found. It does this via a check on installed.packages() before running pkg_ref(). For 3, I added a simple function called fix_src_name() to auto adjust the source type. This should enable users to pass just "cran" or "install" and get the right source type.

[parmsam]

Going to merge to ease_of_use branch. Assuming this is safe since it is not yet in main and PR is approved. Thanks!