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

An rlang-ish equivalent to stopifnot() #1111

Open
jennybc opened this issue Feb 4, 2021 · 8 comments
Open

An rlang-ish equivalent to stopifnot() #1111

jennybc opened this issue Feb 4, 2021 · 8 comments
Labels
feature a feature request or enhancement
Milestone

Comments

@jennybc
Copy link
Member

jennybc commented Feb 4, 2021

Summarizing a slack conversation with @lionel-

I'd be interested in an rlang substitute for stopifnot(). Recently, I've switched all conditions in reprex to rlang. Well, except for places where I have a whole raft of simple stopifnot()s, which are quick, basic checks I don't expect to be triggered by real users very often.

Reasons I'd like to adapt the stopifnot()s to a fictional abort_if_not():

  1. Consistency within the package
  2. Backtraces
  3. A place to get some message-making help à la abort()'s auto bullets or arg_match0(). Even stopifnot() has recently gained some features around this.

I realize, in general, we strive to make better error messages than any stopifnot() can do. But I still think there's a role for simple checks that are sort of pro forma, but that are still useful and are unlikely to be encountered by actual users.

Previous related discussion in #37

@nerutenbeck
Copy link

nerutenbeck commented Feb 18, 2021

We've been trying to figure out best development practices regarding the use of rlang::abort, assertthat::assert_that, and stopifnot in our internal package development while keeping outside dependencies to a minimum, so something along these lines would be really helpful, since we use rlang for all kinds of stuff and love abort for condition catching. For simpler / rarer error checking, though, it feels heavy-handed to import assertthat, even though we like the improved messaging. It has been somewhat difficult to navigate the role / advantages / disadvantages of different error handling contexts and appropriate strategies. Thanks!

@tzakharko
Copy link

To throw another idea in the mix, I personally use a short-circuiting or, e.g.

assertion || abort(...) 

I think it's concise and readable. The drawbacks I can see is that you can get weird stuff when assertion is not a scalar boolean, there are no auto-generated error messages, and it's also marginally slower than an if. As completely crazy idea, how about something like a "magical assertion operator", e.g.

assertion %?% abort(...)

that would validate assertion as well as inject a formatted message into abort if no message was given?

@lionel- lionel- added the feature a feature request or enhancement label Mar 29, 2021
@lionel- lionel- added this to the maybe milestone Apr 1, 2021
@hadley
Copy link
Member

hadley commented Apr 14, 2021

One thing we should definitely copy is the (IIRC relatively new ability) to customise the error message:

stopifnot("m must be symmetric"= m == t(m))

@davidchall
Copy link

In case it helps anyone here, I recently started using {checkmate} instead of {assertthat}.

Pros:

  • Each check gets 3 forms: test_ (return boolean), assert_ (raise error), and expect_ (testthat expectation)
  • Messages can be tailored to what went wrong
  • Checks can be customized (e.g. arguments to allow/disallow NAs and NULLs)
  • Extension system for new checks
  • Lightweight dependencies

Cons:

  • A lot of functions exported (bloated namespace?)
    • Especially since they support snake case and camel case
  • assert_ functions use stop() instead of abort()

@moodymudskipper
Copy link

I would not dislike it if we didn't miss the opportunity not to use the negative form, which I find confusing :).

abort_if_not(!is.numeric(x)) # -> not not, I need a couple seconds to parse this
assert(!is.numeric(x)) # -> I understand right away

or provide an abort_if() counterpart for these cases

@tzakharko
Copy link

tzakharko commented Jul 4, 2022

I have recently uploaded another attempt (currently experimental) at rolling an assertion package (modelled after opinionated assertions in Swift), which might be relevant for this discussion. It uses abort() if rlang is installed, but also offers a type of assertion (sanity check) that will immediately stop the script bypassing any error handling if failed.

https://github.com/tzakharko/precondition

One feature of note is that parts of the assertion expression can be embraced which will print the relevant value as part of the assertion error. E.g.

x <- -5
precondition({{x}} > 0}

`x > 0` is not TRUE
x = dbl -5

@elipousson
Copy link

elipousson commented Jul 14, 2022

I stumbled across this issue last night after searching for examples of any abort_if_not helper functions and put together a basic version that works all right. Curious to hear from more experienced rlang users or contributors about what other approaches are possible to achieve this result using rlang!

library(rlang)

abort_if_not <- function(...,
                         message = NULL,
                         class = NULL,
                         not = TRUE,
                         call = caller_env()) {
  params <- list2(...)

  if (length(params) > 1) {
    for (x in seq(params)) {
      abort_if_not(
        params[[x]],
        message = names(params)[[x]],
        class = class,
        call = call
      )
    }

    invisible(return(NULL))
  }

  condition <- params[[1]]

  if (is.logical(condition) && ((!condition && not) | (condition && !not))) {
    if (is_named(params)) {
      message <- message %||% names(params)
    }

    abort(
      message = message,
      class = class,
      call = call
    )
  }

  invisible(return(NULL))
}


abort_if_not(
  "pass if true" = TRUE
)
#> NULL

abort_if_not(
  "abort if false" = FALSE
)
#> Error:
#> ! abort if false

abort_if_not(
  "pass first if true" = TRUE,
  "pass second if true" = TRUE,
  "abort third if false" = FALSE
)
#> Error:
#> ! abort third if false

abort_if_not(
  "equivalent to abort_if" = TRUE,
  not = FALSE
)
#> Error:
#> ! equivalent to abort_if

Created on 2022-07-14 by the reprex package (v2.0.1)

@MichaelChirico
Copy link
Contributor

One thing we should definitely copy is the (IIRC relatively new ability) to customise the error message:

just for posterity, yes, this was part of R 4.0.0.

and just as a side note to add a little more value with this comment: consider internationalization with this interface, which stopifnot() itself does not yet support

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature a feature request or enhancement
Projects
None yet
Development

No branches or pull requests

9 participants