Ensure packages referenced using :: notation are included in reproducibility table#314
Ensure packages referenced using :: notation are included in reproducibility table#314
Conversation
kelliemac
changed the title
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #314 +/- ##
===========================================
- Coverage 85.33% 84.93% -0.40%
===========================================
Files 9 9
Lines 375 385 +10
===========================================
+ Hits 320 327 +7
- Misses 55 58 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@slager do you have any thoughts on how to extend these unit tests so that the code coverage checks pass? |
R/misc.R
Outdated
| #' @return Character vector of unique package names (may be length 0). | ||
| #' @export | ||
| #' @family utilities | ||
| detect_namespaces_in_file <- function(path){ |
There was a problem hiding this comment.
This function could be deleted, and the other function could just be called with readLines(knitr::get_current_input()) as its argument.
| # ensure packages referenced with :: are loaded before calling get_session_info() | ||
| pkgs_to_load <- detect_namespaces_in_file(knitr::current_input()) | ||
| for (pkg in pkgs_to_load) { | ||
| if (any(installed.packages()[,1] == pkg)) suppressWarnings(library(pkg, character.only = TRUE)) |
There was a problem hiding this comment.
Here and in the other skeleton, I think this approach could use an adjustment (and/or discussion). Loading a package into the search path via library() can produce different behaviors than accessing a package's function using the :: syntax. So, in the attempt to document which packages were used, we'd in effect be creating a reproducibility table that shows a different R environment than what was used in the main script.
One possibility is converting this into a check that throws an error if not all packages called by :: are already loaded into the search path, and let get_session_info() do the rest of the work. This would force users to list their library calls up front, which is kind of a best practice anyway since it's nice to see up near the top what your script depends on. Alternatively, and this would probably be the most accurate, would be to try to parse loaded namespaces and attached namespaces and disambiguate those in the reproducibility table. This would give users more flexibility in how they choose to use library() and pkg::fun() calls.
There was a problem hiding this comment.
that makes sense to me! I will make some adjustments.
kelliemac
changed the title
|
per discussion today, @slager will start a fresh branch / PR with a different approach to list all namespaces and add a column to table indicating if attached or not |
|
Implemented at FredHutch/VISCfunctions#132 |
|
closing in light of FredHutch/VISCfunctions#132 (review) and #329 |
2 participants
Description
Attempt to automatically include packages leveraged in main report Rmd file using
::notation in the reproducibility table at the end of a report.These code changes were mostly done by CoPilot, but carefully reviewed for validity by Kellie.
Related Issues
#211
Checklist
devtools::document