Document how to avoid build-time dependency

[krlmlr]

Following @gaborcsardi's advice in https://stat.ethz.ch/pipermail/r-devel/2017-January/073647.html, I tried memoising in .onLoad(), and it worked for me:

# file.R
fun <- function() {
  some_expensive_process()
}

# zzz.R
.onLoad <- function(pkgname, libname) {
  fun <<- memoise::memoise(fun)
}

This eliminates the build-time dependency on memoise and also gets rid of R CMD check warnings about using memoise without importing it.

I wonder if and where we should document this.

[mdsumner]

Oh magic, thanks! I was looking for this advice in ?memoise and in the readme, and then I searched through Issues to find this. It's a subtley about the package process that I hadn't understood before. Would a short note suffice?

It is recommended that functions in a package are not memoised at build-time, but when the package is loaded. The simplest way to do this is within .onLoad() with, for example <code block above>

That would have worked for me in readme and/or help.

[mdsumner]

Added a PR for this

[dpprdan]

It's a kind of magic indeed.

Could I also update my memoised function at run-time like this?

E.g. I have defined my function fn as mentioned above and now I want to give the user the option to set a different cache. For this I want to use the function config_fn.

The following throws an error, because fn is already memoised.

config_fn <- function(cache = memoise::cache_memory()) {
  fn <<- memoise::memoise(fn), cache = cache))
}

I guess one can use environment(fn)[["_f"]] instead (with the problems mentioned in #36, so this might be a relevant use case for unmemoise(), IMHO)?

config_fn <- function(cache = memoise::cache_memory()) {
  fn <<- memoise::memoise(environment(fn)[["_f"]], cache = cache)
}

Still this does not seem to work - the cache does not get invalidated - so probably my understanding of (recursively updating) environments is flawed?

[dpprdan]

FWIW, @richfitz mentioned elsewhere that he has had a CRAN rejection 'for using <<- for "modifying the global environment" for uses where it does not modify the global environment'.

[gaborcsardi]

@dpprdan No, that's just silly, and he/we need to push back on this. We clearly do not modify the global environment. We modify the package's namespace.

[tanho63]

Appreciated this thread a bunch! I too nearly ran into a CRAN rejection for "global environment" but was able to demonstrate (with the help of this thread and Gabor's previous one) that it does not modify the global environment. I supplied a screenshot to help support it, leaving it here for future me and others:

[tylerlittlefield]

FWIW, @richfitz mentioned elsewhere that he has had a CRAN rejection 'for using <<- for "modifying the global environment" for uses where it does not modify the global environment'.

Bummer, I've just run into this as well :/

[dpprdan]

@tyluRp Is this an initial submission to CRAN or a package update?
It might help to explain in the comments why you are using <<-, like we did here for {opencage}: https://github.com/ropensci/opencage/blob/main/R/zzz.R
This is also in the current CRAN version. It was added with one of the last package updates, though, so chances are that no one looked at the source code.

[tanho63]

Bummer, I've just run into this as well :/

@tyluRp this is fightable - I did end up winning on this! See the screenshot I had, where librarying the package + printing the global environment with all names via ls(all.names = TRUE) provides a clean env.

Also explain that you're modifying the package NS on load and nothing else?

[tylerlittlefield]

@tanho63 @dpprdan I ended up responding to the email with similar verbiage "this is modifying the packages namespace, not the global environment" and they sent my package to CRAN!! A pleasant surprise :)

[tanho63]

Coming back to this after discovering rlang::ns_env() which imo makes this easier to explain to others, e.g. CRAN

cache_function <- function(function_name, duration = 86400, omit_args = c()) {
  fn <- get(function_name, envir = rlang::ns_env("ffscrapr"))
  fn <- memoise::memoise(
    fn,
    ~ memoise::timeout(duration),
    omit_args = c(),
    cache = cachem::cache_memory()
  )
  assign(function_name, fn, envir = rlang::ns_env("ffscrapr"))
  return(invisible(TRUE))
}
.onLoad <- function(libname, pkgname) {
  lapply(c("ff_rosters", "mfl_players"), cache_function)
}

[jimbrig]

Coming back to this after discovering rlang::ns_env() which imo makes this easier to explain to others, e.g. CRAN

cache_function <- function(function_name, duration = 86400, omit_args = c()) {
  fn <- get(function_name, envir = rlang::ns_env("ffscrapr"))
  fn <- memoise::memoise(
    fn,
    ~ memoise::timeout(duration),
    omit_args = c(),
    cache = cachem::cache_memory()
  )
  assign(function_name, fn, envir = rlang::ns_env("ffscrapr"))
  return(invisible(TRUE))
}
.onLoad <- function(libname, pkgname) {
  lapply(c("ff_rosters", "mfl_players"), cache_function)
}

This is great, I tweaked it a little bit to allow for assigning separate "prefixed" memoised functions if that fits your use case:

# internal ----------------------------------------------------------------

#' @keywords internal
#' @noRd
#' @importFrom memoise memoise timeout
#' @importFrom cachem cache_memory
#' @importFrom rlang ns_env
#' @importFrom glue glue
#' @importFrom cli cli_alert_info
.cache_function <- function(
  function_name,
  pkg,
  duration = 86400,
  omit_args = c(),
  cache = cachem::cache_mem(),
  rename_prefix = "mem_",
  quiet = TRUE,
  ...
) {

  fn <- base::get(function_name, envir = rlang::ns_env(pkg))

  mem_fn <- memoise::memoise(
    fn,
    ~ memoise::timeout(duration),
    omit_args = omit_args,
    cache = cache
  )

  mem_function_name <- glue::glue("{rename_prefix}{function_name}")

  assign(mem_function_name, mem_fn, envir = rlang::ns_env(pkg))

  if (!quiet) {
    cli::cli_alert_info("Created a cached function for {.field {function_name}} as {.field {mem_function_name}}.")
    cli::cli_alert_info("The cache will expire in {.field {duration}} seconds.")
  }

  return(invisible(TRUE))
}

# onLoad ------------------------------------------------------------------

#' @keywords internal
#' @noRd
#' @importFrom purrr walk
.onLoad <- function(libname, pkgname) {

  # cache functions ---------------------------------------------------------
  c(
    "get_entrata_reports_list",
    "get_entrata_report_info",
    "get_latest_report_version",
    "get_property_ids_filter_param"
  ) |>
    purrr::walk(.cache_function, pkg = pkgname, quiet = FALSE)

}