funecology / fundiversity

📦 R package to compute functional diversity indices efficiently
https://funecology.github.io/fundiversity/
GNU General Public License v3.0
32 stars 3 forks source link

Be more specific in the help with memoization #77

Closed Rekyt closed 7 months ago

Rekyt commented 1 year ago

A little connex to #71 but the current description of memoisation https://github.com/funecology/fundiversity/blob/2d19f5c5b2b52f58323ce0cd89e5a04547c11813/R/fd_fric.R#L34-L41 doesn't mention that the options should be used before load fundiversity.

Maybe it could also be good to mention when to use and not use memoisation.

Bisaloo commented 1 year ago

This is a good point and I wonder if it's more than a documentation issue. It seems cumbersome and error-prone to have to restart your session. Would it work & make sense to read this option in the functions calling fd_chull() and fd_chull_intersect()?

Rekyt commented 12 months ago

The problem is the way we implement memoization. To make it work seamlessly, we check the option when loading the package. Then we load the updated version of the functions. https://github.com/funecology/fundiversity/blob/b98a6fef9f309e4837ffae97083c9ef2cf7054b9/R/zzz.R#L4-L10

Our initial goal was to make it as transparent as possible for users to use memoise. So I'm not sure implementation wise how to keep that while changing the way we check the options and load the functions.

Should we have an overarching function (like fd_chull()) that checks the option and overloads internal functions?

Something in the line of:

fd_chull <- function(...) {

  if (requireNamespace("memoise", quietly = TRUE) && 
       isTRUE(getOption("fundiversity.memoise", TRUE))) {
    fd_chull_memoised(...)
  } else {
    fd_chull_unmemoised(...)
  }
}