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

add combination of is_installed() and check_installed() #1730

Open
2 tasks
maxheld83 opened this issue Jul 6, 2024 · 4 comments
Open
2 tasks

add combination of is_installed() and check_installed() #1730

maxheld83 opened this issue Jul 6, 2024 · 4 comments

Comments

@maxheld83
Copy link
Member

I sometimes find myself in this situation:

if (rlang::is_installed("optional-package")) {
  # do something more fully-featured
} else {
  # do a simpler version without the opt package
}

Typically, such an optional-package will be a Suggests:.

If they're missing it, I'd like to inform my users about them missing optional-package and (once) prompt them to install it.
rlang::check_installed() is great for this except that it errors out.

So I'd like to us a "combination" (loosely speaking) of the two functions which:

  • always returns as a predicate would (TRUE/FALSE)
  • if interactive, prompt the user to install
  • if non-interactive inform the user that they're missing

If you deem this useful, read on -- and I'll be happy to write up a PR with some guidance --,
otherwise feel free to close 🙂

I've build a hacky version of this (see below), but ran into these limitations which may require/justify inclusion/deeper integration in rlang:

  • just catching and re-signaling (as a message) check_installed() does not work great, because:
    • the wording (~ "required package missing") of the condition doesn't quite work as a message; it sounds too alarming and may confuse users.
    • I can't use .frequency for how often to even prompt the user (which may be even more annoying than messages, if done too often). (related let cnd_signal() take .frequency arg #1729)
  • rebuilding based off of only is_installed() would duplicate a lot of work already done in rlang.

In summary, it might be nice to include this feature in rlang, though to avoid code duplication, it might require some refactoring.

Also some unresolved questions:

  • how should this function be named?
    It's too side-effecty for is_installed().
    Something like is_installed_after_trying()?
    Is there a idiomatic name for this kind of pattern?
  • is it ok to reuse rlang:::needs_signal() for when to try installing?
    That clearly needs a .frequency not to be annoying, rlang:::needs_signal() isn't about when/how often to prompt.
@teunbrand
Copy link
Contributor

I'm running into the exact same scenario.
I couldn't for the life of me figure out how to combine try_fetch() + check_installed() in a way that would produce the desired behaviour.

@maxheld83
Copy link
Member Author

@teunbrand in case that's helpful I got this to work:

#' Checks if a package is installed and *informs* the user if not
#'
#' This is wrapper around [rlang::check_installed];
#' instead of erroring out if the check fails it returns `FALSE`.
#' However, unlike [rlang::is_installed], it emits a message to the user.
#'
#' @inheritParams rlang::check_installed
#' @inheritDotParams rlang::check_installed
#' @example inst/examples/dependencies/is_installed2/missing.R
#' @example inst/examples/dependencies/is_installed2/present.R
#' @keywords dependencies helper
#' @export
is_installed2 <- function(...) {
  if (rlang::is_installed(...)) {
    return(TRUE)
  }
  rlang::try_fetch(
    # TODO this should only interact with the user as per .frequency
    # might get annoying otherwise
    # but that is blocked by deep integration in rlang
    rlang::check_installed(...),
    error = function(cnd) {
      inform_missing_pkgs(...)
    }
  )
  rlang::is_installed(...)
}

you can replace inform_missing_pkgs() with your own simpler rlang::inform("blah").

Full source, largely copied from rlang itself is here: https://github.com/dataheld/elf/blob/main/R/dependencies.R

Though it's all a bit hacky, not ready for prime time.

@teunbrand
Copy link
Contributor

teunbrand commented Jul 11, 2024

Thanks for sharing this code, Max!
This is similar to what I attempted but I found one gripe with this approach.
If a user selects 'No' to a prompt, the function should return FALSE but instead no value is returned at all.
It is hard to provide a reprex as this only occurs in interactive sessions, but essentially:

x <- is_installed2("foobar")
# User should select 'No' as answer
print(x)
#> Error: object 'x' not found

@olivroy
Copy link
Contributor

olivroy commented Jul 11, 2024

Related #1658. In dm there is a standalone file that basically extends the is_installed() behavior to skip in tests.

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

3 participants