Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Factor out needs_packages() #102

Closed
salim-b opened this issue Jun 6, 2020 · 2 comments
Closed

Factor out needs_packages() #102

salim-b opened this issue Jun 6, 2020 · 2 comments

Comments

@salim-b
Copy link
Contributor

salim-b commented Jun 6, 2020

I just submitted #101 and now I'm wondering: Are there any plans on factoring out the needs_packages() function into a separate package? Or maybe introduce something similar in pak?

Currently, the function looks like this:

pkgsearch/R/utils.R

Lines 100 to 127 in 9c0102e

needs_packages <- function(pkgs) {
has <- map_lgl(pkgs, function(pkg) {
requireNamespace(pkg, quietly = TRUE)
})
if (!all(has)) {
not_installed_pkgs <- pkgs[!has]
if (length(not_installed_pkgs) == 1) {
throw(new_error(
"The ",
sQuote(not_installed_pkgs),
" package is needed for this addin.",
call. = FALSE
))
} else {
throw(new_error(
"The ",
paste(sQuote(not_installed_pkgs), collapse = ", "),
" packages are needed for this addin.",
call. = FALSE
))
}
}
}

This implementation of course is not very sophisticated and it could be improved (i.a. provide an auto-generated code snippet to install the missing packages in the error message).

But it still provides a frequently used check: Test if package xyz, which is only suggested but not imported, is installed and if not, throw an informative error. I would really like to have a "standardized" way to do this!

@gaborcsardi
Copy link
Contributor

Hi, yeah, I would not export this from pkgsearch.

It seems like you want to use this programmatically. I think introducing a new dependency for these 10 lines of mostly trivial code is probably not worth it, so I would say just copy this function into your package(s).

FWIW, there is also rlang::is_installed()

@salim-b
Copy link
Contributor Author

salim-b commented Jun 8, 2020

yeah, I would not export this from pkgsearch.

Sure, I didn't really mean to export it in the current form, sorry for being a bit unclear. I was rather asking if there are any plans on your side (the people around tidyverse, r-lib, r-hub, RStudio etc.) on introducing such a function in a new (or existing, e.g. pak) package in the near future. 🙂

After all, it's an action that's frequently used in a lot of packages and it would be cool to have a best practice implementation in one place (kinda like rlang::arg_match() for argument matching).

But I guess your answer means there are no such plans, right?

FWIW, there is also rlang::is_installed()

Thanks for the hint! I just created r-lib/rlang#991 to make it vectorized... I hope you're not about to rip it right out of the air. 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants