Base `PEcAn.logger` on native R functions

[ashiklom]

Description

Mask R's message, warning, and stop functions in PEcAn.logger as the following:

message <- PEcAn.logger::logger.info
warning <- PEcAn.logger::logger.warn
stop <- PEcAn.logger::logger.severe

And, start replacing all occurrences of PEcAn.logger::logger.* with the native R functions. Eventually, we can move PEcAn.logger to Suggests.

This would mean that loading PEcAn.logger in a session (with library(PEcAn.logger)) will trigger the function substitution -- otherwise, running code will default to standard R messaging functions.

Context

A few reasons:

• PEcAn.logger::logger.* is long and annoying to type.
• PEcAn.logger introduces another non-CRAN dependency to every one of our packages, which makes them more annoying to use independently of PEcAn.
• Current logger functions do not work with testthat's expect_error, etc. functions
• They also don't work with suppressWarnings or suppressMessages
• This change would not affect the behavior of functions inside other packages that call message, warning, or stop, because they use their own internal package namespace.

Possible Implementation

See above (each of those would have to be #' @export-ed. Also, the grepl regular expression in PEcAn.logger would need to be tweaked to ignore warning, message, and stop calls to remain informative.

[robkooper]

Main reason was a was a way to collect messages in a file easily. How would that be done with message, warning and stop?

[ashiklom]

So all the logging would work the same way as it does now if the PEcAn.logger package is loaded. It's just that using the logging functions is no longer required. Effectively, what I'm proposing is:

# These call the default R functions
message("hello")
warning("whoops")
stop("whoah")

library(PEcAn.logger)
# By loading the logger package, the functions become masked so
message("hello")      # This actually calls `PEcAn.logger::logger.info
warning("whoops")   # This calls `PEcAn.logger::logger.warn
stop("error")      # This calls `PEcAn.logger::logger.severe

So all the same functions for setting logger level, output file, etc. should work the same way.

On a side note, base R's message, warning, and stop can be selectively redirected via sink.

[infotroph]

Re names being too long: I'd favor exporting aliases that drop the redundant logger.* part, e.g. PEcAn.logger::info(). Anything beyond that gets back into the argument about namespaces vs library calls.

Re awkwardness with with functions like expect_error or suppressMessages: The big underlying difference here is that PEcAn.logger doesn't use R's formal condition-signaling mechanism, and is therefore awkward to use with any function that expects a condition. If PEcAn.logger can be revamped to use conditions, I'd favor doing that, but I'm also not familiar with the history here -- @robkooper, was avoiding signalCondition an intentional design choice / necessary to handle some redirection cases?

[ashiklom]

After thinking about this some more, I agree that my approach doesn't make sense, and wouldn't actually work. Personally, I would probably prefer to use R's default functions (stop, warning, message), redirecting them via sink when necessary, but I can see the rationale for sticking with PEcAn.logger. Another possibility is switching to futile.logger, which has been suggested previously (#1362).

In the meantime, I like both of @infotroph 's suggestions -- adding shorter aliases and signaling conditions.

[ashiklom]

Another promising idea is the debugme package, which provides syntax like this:

f <- function(a, b = 3) {
  c <- a + b
  d <- a * b
  e <- a ^ b
  "!DEBUG arguments are a = `a` and b = `b`"
  c(c, d, e)
}

[github-actions[bot]]

This issue is stale because it has been open 365 days with no activity.

[infotroph]

See also discussion in #1362