Include the name of the object in the error message

[gaborcsardi]

I promise that this is the last one for today. :)

As much as I like the dot notation, I think it would be good to see that name of the object that breached the contract (if it has a name at all). Often the name is informative. I would not replace the dot itself, but I think something like this looks good and is more informative:

ensure_small_even <- ensures_that( . < 50, . %% 2 == 0)
f <- function() {
   value <- 101
   ensure_small_even(value)
}
f()
Error:  assertions failed for 'value' in 'f':
     ✖ all(. < 50)
     ✖ all(. %% 2 == 0)

It might be challenging to implement this, though. In the ensurer function, deparse(sys.call(-1)[[1]]) gives the function name, and deparse(sys.call()[[2]]) gives the argument name, this is easy.

But of course you can have anonymous functions, and you can also wrap ensurer functions, in which cases this is more difficult. You would probably need API to wrap an ensurer function. E.g. I am doing this now:

ensure_character_scalar <- function(x) {
  x %>%
    ensure_that(is.character(.)) %>%
    ensure_that(length(.) == 1)
}

Which is not bad, but ensure_character_scalar is not an ensurer function, just a plain one.

And other strange cases might exist, that I am unaware of.

[smbache]

What should be the msg for

something %>%
  manipulation_1 %>%
  ...
  manipulation_n %>%
  ensure_that( ... )

[gaborcsardi]

Yeah, I was thinking about that, too. If you have something like this:

ensurer <- function(x, y=1, z=5) {
  print(sys.status())
  browser()
}

g <- function() {
  100 %>%
    add(10) %>%
    ensurer() %>%
    print()
}

g()

Then the printed call stack is this:

$sys.calls
$sys.calls[[1]]
g()

$sys.calls[[2]]
100 %>% add(10) %>% ensurer() %>% print()

$sys.calls[[3]]
eval(lhs, env)

$sys.calls[[4]]
eval(expr, envir, enclos)

$sys.calls[[5]]
100 %>% add(10) %>% ensurer()

$sys.calls[[6]]
withVisible(eval(e, env, env))

$sys.calls[[7]]
eval(e, env, env)

$sys.calls[[8]]
eval(expr, envir, enclos)

$sys.calls[[9]]
ensurer(.)

$sys.calls[[10]]
print(sys.status())

$sys.calls[[11]]
sys.status()
...

So with some internal knowledge about magrittr, you can go back to add(10), which produced the value that breached the contract. (I guess you just need to jump over eval and withVisible, and if the next kind of function is %>%, then you can report something meaningful.

But this might be too ambitious or simply stupid, I do not know enough about NSE and magrittr to judge. I guess it is also reasonable not to report a function or a name in this case.

[gaborcsardi]

Btw. I realize that handling all special cases is most probably impossible. But I think printing something meaningful in the (more) common cases is still useful. If at all possible in a reasonable way.

[smbache]

I think it is a bad idea to rely on specific forms of evaluation, even magrittr's. You could see something like

ensure_that(which(f(x) %>% is.na), ...)

or many other constellations. I think this would complicate things a lot, at high risk.

[gaborcsardi]

Fair enough.

[smbache]

@gaborcsardi Try the recently commited version. Would this be satisfying to you?

Basically adds sys.call(1L) to the message, but shortens it if it is too long (error line will have nchar <= 80)

If it is too long, it will print the beginning and end with ... in the middle. Example:

> iris %>% ensure_that(is.data.frame(.), is.logical(.))
 Error: conditions failed for call 'iris %>% ensure_th ... .), is.logical(.))':
     * is.logical(.)

> iris %>% ensure_that(is.logical(.))
 Error: conditions failed for call 'iris %>% ensure_that(is.logical(.))':
     * is.logical(.) 

[gaborcsardi]

I think this is good now. Now that there is useful information in the first line, I don't even want to remove it.

We'll see if we want to tweak it more (e.g. use getOption("width")), but I think you can close this issue now. Thanks.

[smbache]

Yeah, maybe thats a good idea