search

tidyfun / tf

S3 classes and methods for tidy functional data
https://tidyfun.github.io/tf/
GNU Affero General Public License v3.0
5 stars 2 forks source link

Signal an error, warning, or message #70

Closed m-muecke closed 7 months ago

m-muecke commented 7 months ago

Currently rlang is already used in the package, thus I would vote for either using rlang::abort, etc. instead of stop and warning for nicer errors or not displaying the call with stop(call. = FALSE) and warning(call. = FALSE) https://design.tidyverse.org/err-call.html.

Example for using it with stop in the lintr package: https://github.com/r-lib/lintr/blob/main/R/comments.R#L87

fabian-s commented 7 months ago

:+1:

m-muecke commented 7 months ago

👍

whats your preference?

fabian-s commented 7 months ago

we can/should move to exception handling with rlang functions, seems standard for tidyverse packages. thank you for pointing that out!

fabian-s commented 7 months ago

on 2nd thought: we use checkmate everywhere and checkmate does not use rlang exceptions anyway, so let's stick with stop(call. = FALSE) and warning(call. = FALSE)

m-muecke commented 7 months ago

on 2nd thought: we use checkmate everywhere and checkmate does not use rlang exceptions anyway, so let's stick with stop(call. = FALSE) and warning(call. = FALSE)

Sounds good. With rlang::abort you would've to add the environment for all errors in the helper functions anyways, leading to quite a bit of modification otherwise you would still have the entire stack or I guess just setting the call to NULL would've done the same. This would've been the pattern in non exported functions:

foo <- function(..., error_call = caller_env()) {
  abort("error", call = error_call)
}