Extend a contract

[gaborcsardi]

I have an ensurer function that I want to extend, ideally without writing out the full original contract again. I think currently the best I can do is putting the original and the new contracts together with a pipe (%,% or %>%). This is not bad, but unfortunately behaves slightly differently, as it does not list all errors, if there are some both in the original contract and in the additional one.

Would you consider some API to extend a contract?

[smbache]

I definitely considered this. Not sure what syntax would be best though...

[smbache]

Suppose you have two contracts, A and B, and you want to combine them. What should happen if fail_with is different for the two?

Another option would be to let ensure(s)_that accept contracts as arguments, which would then extract the conditions (but not fail_with) and reuse them, then one could do

something %>%
    ensure_that(length(.) > 0, +A, +B, fail_with = NA_real_)

I am thinking I need to prepend something like + to distinguish contract arguments.

[gaborcsardi]

Actually, I was thinking about this, and I don't really want this any more. :) Often one check only makes sense if a previous one already passed, so I am not even sure that I want to perform (and report) all checks, at all.

So, I am OK with just putting the checks together with a pipe. This is elegant, and you don't need to add any more syntax.

[gaborcsardi]

As for your proposed solution, I think it is good to have contracts as arguments. Maybe you don't even need the + sign, but just check the class of the object. (This probably requires that you evaluate the argument twice, once with standard, and once with non-standard evaluation.) Anyway, as I said before, I don't really need this any more, so we can think about it more.

Btw. the other problem I see with merging contracts is that they might come from checks of different objects. E.g. in the ensure_has_attr case, you can have something like:

ensure_has_attr <- function(x, attr) {
  ensure_character_scalar(attr)
  ensure_that(x, attr %in% names(attributes(.)))
}

and then I probably don't want to perform the second check at all, if the first one fails. So here it is better to put them in a sequence, instead of merging the two contracts. I think it is true in general, that some checks only make sense if others succeeded. E.g. even just

ensure_that(is.character(.), length(.) == 1)

makes more sense as

ensure_That(is.character(.) && length(.) == 1)

I think.

[smbache]

With the newest commit, you can now do (not so realistic but to show the point):

A <- ensures_that(is.data.frame(.), nrow(.) > x, x = 50)
B <- ensures_that(ncol(.) > y, y = 3)
C <- ensures_that(all(.[, 1] > 0), +A, +B)

> C
Ensures that
    * all(.[, 1] > 0) 
    * is.data.frame(.) 
    * nrow(.) > x 
    * ncol(.) > y 

Also, x and y are made available to C.

I wouldn't want to evaluate things twice, and I think the + makes intuitive sense as well.

How's that?

[gaborcsardi]

Looks pretty good, actually. I think this will do, and we can close this.