1012 substitute {assertthat} with {checkmate} in a couple of functions

[m7pr]

Part of #1012
Removed {assertthat} package and substituted with {checkmate} equivalents.
Removed R/dynamic_assertions.R since functions are not used in this package, and not in any other place in pharmaverse.

[averissimo]

TODO: need help removing assertthat::on_failure in R/dynamic_assertions.R

I think it's a dynamic helper that helps to assert function results without explicitly calling the validation function. (it adds an attribute to the function)

It also creates a nice verbose message as a benefit

I don't think {checkmate} supports this. An equivalent example would be:

expect_error(validate_enough_rows(teal_enough_rows(data = iris, min_nrow = 1500)))

With assert that you may not skip using validate_enough_rows function explicitly)

library(testthat)
library(assertthat)

teal_enough_rows <- function(data, min_nrow) nrow(data) >= min_nrow

validate_enough_rows <- function(data, min_nrow) {
  shiny::validate(
    shiny::need(
      FALSE,
      label = paste0(
        substitute(data),
        ": Minimum number of records not met: >= ", min_nrow,
        " records required."
      )
    )
  )
}

teal_enough_rows(data = iris, min_nrow = 1500)
#> [1] FALSE
assert_that(teal_enough_rows(data = iris, min_nrow = 1500))
#> Error: teal_enough_rows(data = iris, min_nrow = 1500) is not TRUE

attr(teal_enough_rows, "fail") <- function(call, env) {
  call[[1]] <- validate_enough_rows
  eval(call, envir = env)
}

teal_enough_rows(data = iris, min_nrow = 1500)
#> [1] FALSE
assert_that(teal_enough_rows(data = iris, min_nrow = 1500))
#> Error: iris: Minimum number of records not met: >= 1500 records required. must be provided

^{Created on 2024-02-07 with reprex v2.0.2}

[github-actions]

Unit Tests Summary

  1 files   33 suites   2s ⏱️
150 tests 150 ✅   0 💤 0 ❌
282 runs  170 ✅ 112 💤 0 ❌

Results for commit 970096e.

♻️ This comment has been updated with latest results.

[m7pr]

@averissimo I don't think I understood fully your comment but I get that we are not able to substitute this part with anything from checkmate right? I checked and neither of teal_enough_rows, validate_enough_rows, teal_has_element, validate_has_elements is used in the package so maybe we can delete them?

[m7pr]

@Melkiades are you aware if

teal_enough_rows, validate_enough_rows, teal_has_element, validate_has_elements

are used anywhere outside of this package? We were hoping we can remove those, or at least move to /dev : )

[averissimo]

Minor suggestions, overall looks really good!

[m7pr]

I removed R/dynamic_assertions.R in c81fee9 since functions are not used in this package, and not in any other place in pharmaverse.

[m7pr]

Looks like 86 tests are failing 👯

[m7pr]

Looks like 86 tests are failing

Should be fixed by now

[averissimo]

Looks good to merge, however I think an assertion below will not work, take a look at one of the comments

if (isTRUE(interact_y)) all(sapply(interact_y, checkmate::assert_string))

I also discovered that you can place an if clause inside assert() function and get pretty messages (without having to create a check function) 🥳

[Melkiades]

Looks good to merge, however I think an assertion below will not work, take a look at one of the comments

if (isTRUE(interact_y)) all(sapply(interact_y, checkmate::assert_string))

I also discovered that you can place an if clause inside assert() function and get pretty messages (without having to create a check function) 🥳

Looking at it now, it is a character vector, so I would just do checkmate::assert_character(...)

[Melkiades]

For me it is good to go. Thanks @m7pr!! ;)

adressed