mllg / checkmate

Fast and versatile argument checks
https://mllg.github.io/checkmate/
Other
261 stars 30 forks source link

Add subclasses to conditions #221

Open markschat opened 2 years ago

markschat commented 2 years ago

If I´m not mistaken checkmate is not throwing conditions with subclasses at the moment (Hadely Wickham has a great summary on this topic here). I very much would appreciate to see this implemented in checkmate. I see this advantages (but there might be more):

First: Checkmate can improve it´s messages without breaking any tests in other packages (at the moment most likely the condition$msg is parsed in tests, which is not very clean I feel).

library(testthat)
library(rlang)

# checking without subclass
expect_error(my_func(), "Specific error message thrown by assert_that, that might change")

# checking with subclass
err <- catch_cnd(my_func())
expect_s3_class(err, "assertion_error") # Checking error class, instead of message

Second: Conditions/ Errors can be caught specifically by other functions (I very much in love with this idea).

tryCatch(
  assertion_error= function(cnd) "bad_argument", # catch error by subclass
  error = function(cnd) "other error",
  my_func()
)

I just wanted to kickoff a discussion on this topic. I think this feature can be integrated cleanly without breaking any existing code.

mllg commented 2 years ago

This actually was in checkmate some years ago. I've removed it once base R started to introduce custom error classes and I wanted to wait for them to settle. For some reasons, they stopped after packageNotFoundError.

Reintroducing a generic assertion_error should be no problem. Specializing this more means a lot of work, though.

markschat commented 2 years ago

This actually was in checkmate some years ago. I've removed it once base R started to introduce custom error classes and I wanted to wait for them to settle. For some reasons, they stopped after packageNotFoundError.

Didn´t know about that and wonder why base R isn´t pushing this topic anymore. For me error handling in R is one of the biggest weak points and feels very sloppy compared to all other languages I used so far. Catching all kinds of exceptions is considered bad practise in most other languages (although I´ve only worked with OO languages so far).

rgayler commented 1 year ago

I am just encountering this issue now (from a zero prior expertise base). I have checkmate assertions in my package code for argument checking and testthat::expect_error() in my unit testing.

I would like to follow the testthat advice to use the regexp = or class = arguments to ensure that expect_error() only detects errors from the checkmate assertions.

In the absence of an error class from checkmate I am using regexp = but the regular expression depends on the exact wording of the stop() message in the checkmate assertion, which seems a bit prone to false matches.

A generic checkmate_assertion_error would seem safer, in that it specifically mentions checkmate as the source of the error.