assertthat icon indicating copy to clipboard operation
assertthat copied to clipboard

feature request: assert_that_any & assert_that_all to "vectorise" assert_that

Open gowerc opened this issue 7 years ago • 4 comments

Quite often I find myself needing to check a vector of conditions for example

library(assertthat)
x <- c(1,2,3,4)
assert_that( x >= 1)

which fails because x is not of length 1

To get around this I often define the following utility functions

assert_that_any <- function(...){
    x <- list(...)
    for( i in x){
        assert_that(any(i))
    }
}

assert_that_all <- function(...){
    x <- list(...)
    for( i in x){
        assert_that(all(i))
    }
}

assert_that_any(
    c(1,2,3,4,5) <= 5, 
    c(1,1,1,2,2) == 1
)

assert_that_all(
    c(1,2,3,4,5) <= 5, 
    c(1,1,1,2,2) == 1
)

It would be awesome if this could be included into the library (happy to attempt a PR if you agree)

gowerc avatar Sep 10 '18 16:09 gowerc

Why not just do this?

assert_that(
    any(c(1,2,3,4,5) <= 5), 
    any(c(1,1,1,2,2) == 1)
)

It's barely any more typing.

hadley avatar Sep 10 '18 18:09 hadley

I appreciate this must seem like quite a trivial request, the main reason is to keep code concise and avoid nesting of functions which reduces the clarity of the code.

assert_that_all( c(1,1,1,2) <= 1) 

assert_that(all(c(1,1,1,2) <= 1))

To me the top one is just cleaner, easier to read and edit

Also helps when you have a lot of conditions

assert_that_all(
     conds1,
     conds2,
     conds3,
     conds4,
     conds5, 
     ...
)


assert_that(
     all(cond1),
     all(cond2),
     all(cond3),
     all(cond4),
     all(cond5), 
     ...
)

The idea came from a recent training course introducing people to assertthat and testthat where the attendees were relatively new R programmers. People seemed to struggle with the fact assertthat is not vectorised and the most common solution people used for this was:

for ( i in c(1,1,1,2)){
   assert_that( i <= 2) 
}

It is easy to point out that they just require more training on the language fundamentals but I assumed a nicer way would be to have some basic vectorisation support which could be listed in the help page for assert_that.

Again I do appreciate that this is a fairly minor change so feel free to close if you don't believe this adds value.

gowerc avatar Sep 11 '18 08:09 gowerc

I don't understand why you are making this sort of assertion at all, let alone so many of them. Do you mind providing some additional context?

hadley avatar Sep 11 '18 12:09 hadley

Sure, In my current job a very common operation to calculate the difference in days from two character dates. Due to historical reasons the definition of this derivation means that the difference can't be 0. So we end up with something like this:

library(lubridate)
library(stringr)

study_day <- function( date_a,  date_b){ 
    assert_that_all(
        is.character(date_a),
        is.character(date_b),
        str_length(date_a) == 10,  # Can replace with any arbitrary condition to ensure is valid format
        str_length(date_b) == 10
    )
    x <- difftime( ymd(date_a), ymd(date_b), units="days") 
    x <- ifelse(x >=0 , x + 1 , x)
    
    assert_that_all( x != 0)
    return(x)
}

study_day(
   date_a = c("2015-05-01" , "2015-05-03", "2015-05-06"),
   date_b = c("2015-05-01" , "2015-05-04", "2015-05-10")
)

Note that is.character() returns just a single value but the current construct allows for single values as well as vectors to be combined in a single call to assert_that

EDIT:
I am now wondering if it would be better to just incorporate this directly into assert_that() and provide an additional arugment that specifies how to collapse vectors of logical values i.e

assert_that(
    c(1,2,2) ==  2 ,
    is.numeric(1),
   .collapse= all
)

gowerc avatar Sep 11 '18 17:09 gowerc