Add `eval_tidy()` version with default

[clauswilke]

In ggplot2, we extensively use the pattern <data> %||% <default value>, as in color <- data$color %||% "red". This works great when we know what column we want to look up in the data frame, but it fails if we want to do this with eval_tidy(), because eval_tidy() fails instead of returning NULL.

In my own code, I've worked around this by defining a function eval_default() which works like eval_tidy() but swaps in a default value if eval_tidy() fails:
https://github.com/wilkelab/cowplot/blob/06aeeb449ccc0fb343eb03c83111df7b171e73af/R/key_glyph.R#L145-L154

library(rlang)

eval_default <- function(x, data, default) {
  force(default)
  
  suppressWarnings(
    tryCatch(
      error = function(e) default,
      eval_tidy(x, data)
    )
  )
}

data <- data.frame(x = 1)
eval_default(quo(x), data, NULL)
#> [1] 1
eval_default(quo(y), data, NULL)
#> NULL

^{Created on 2020-01-12 by the reprex package (v0.3.0)}

Based on the conversation here, it seems I'm not the only one running into this issue, so maybe adding something like this function to rlang is justified.

One comment regarding my proposed implementation: I had to add suppressWarnings() to prevent a warning "restarting interrupted promise evaluation" that would sometimes pop up. I find this warning sufficiently disconcerting that I'm not sure I can just ignore it, but I don't know enough about the internals of R evaluation to understand what the warning means and whether it can be avoided instead of suppressed.

[lionel-]

I think this would encourage brittle and hacky programming patterns.

[yutannihilation]

But, I think it's a nature of R's lexical scoping that we cannot know whether an evaluation will success or not without trying actually evaluating it. In fact, we already see the pattern at least in ggplot2, cowplot and gghighlight. I believe this is worth adding in rlang.

[clauswilke]

I think my customizable key-glyph function is a good test case because it's rather short and simple:
https://github.com/wilkelab/cowplot/blob/master/R/key_glyph.R

Is it possible to write a similar function without using eval_default()? Or would you argue the function has some fundamental problems and shouldn't exist as written?

[lionel-]

I think all these aes should take their default values in their signature.

Then if the user supplies a wrong expression, they will correctly get an error rather than rectangle_key_glyph() hiding it and giving a default.

[yutannihilation]

It's not that the supplied expression is wrong and we want to hide the errors. We just want to skip the evaluation where the expression cannot be evaluated.

For example, to explain tidyverse/ggplot2#2963 in short, we want to skip splitting the layer whose data doesn't contain the variable to determine PANEL, but this cannot be done by investigating the expression. In this case, the expression is valid, but not on all the layers.

I think this is not a rare scenario because one of the purposes of capturing the expression is to evaluate it in multiple contexts.

[lionel-]

Thanks for the context, I see how this is useful. FWIW, I think the correct approach is to explicitly wrap eval_tidy() in tryCatch(). Ignoring errors in this way is really a stopgap rather than a principled idiom because this swallows programming errors made by the user. Making the tryCatch(error = function(...) default) explicit should have the effect of raising eyebrows when reading/reviewing code, which seems appropriate here.

[lionel-]

On the ggplot2 side, would it be possible to bind the same symbols inside each panel, and when the symbols are undefined use an active binding to throw a typed error? And then you'd only catch these particular errors in the tryCatch(). It sounds like this would produce more principled evaluation semantics.

[yutannihilation]

Thanks, sounds reasonable.

would it be possible to bind the same symbols inside each panel, and when the symbols are undefined use an active binding to throw a typed error?

Cool. I didn't come up with the idea... The remaining questions is whether we should support referring to the variable without symbol (e.g. get("x")). Anyway, I'm gradually feeling that each package should define their own wrapper of eval_tidy() & tryCatch() as you suggested.

[lionel-]

whether we should support referring to the variable without symbol (e.g. get("x"))

With an "undefined" active binding, this would have the same consequence as normal evaluation:

env_bind_active(current_env(), var = function() cat("hello\n"))

var
#> hello

get("var")
#> hello
#> NULL

[clauswilke]

I think all these aes should take their default values in their signature.

Yeah, this is also not possible in my case either. Legend glyphs should be drawn correctly for as many geoms as possible, and it's generally not known (in particular to the user!) which geoms provide which aesthetics to the legend drawing code. So the only meaningful action to take is to fill in default values if and only if the required aesthetics are missing.

In any case, I'm not so worried about the tryCatch() and more about the warning "restarting interrupted promise evaluation". Is there a way to avoid this other than suppressWarnings()? I don't understand the comment about active binding, so I'm not sure whether this applies in my case or not. I'll say, though, that I have no control over what data is fed into the key drawing code, I just have to accept whatever I'm given and make the best of it.

[clauswilke]

After pondering this some more, let me try to make a case for a slightly different behavior: A variant of eval_tidy() where missing objects are substituted with NULL instead of throwing an error. The reprex below explains why. In many cases, we're using eval_tidy() essentially just to look up columns in a data frame, but we don't get equivalent behavior if a missing column is encountered. This means that even if I provide the correct default value in the normal expression we would use (e.g., y %||% 0), the evaluation fails.

library(rlang)

get_aesthetics_value <- function(aes = x, data) {
  aes <- enquo(aes)
  
  eval_tidy(aes, data)
}

data <- data.frame(x = 1)

# try to access `y` directly from data frame
data$y # without default
#> NULL
data$y %||% 0 # with default
#> [1] 0

# try to access `y` via function
get_aesthetics_value(y, data) # without default
#> Error in eval_tidy(aes, data): object 'y' not found
get_aesthetics_value(y %||% 0, data) # with default, breaks
#> Error in is_null(x): object 'y' not found

^{Created on 2020-01-13 by the reprex package (v0.3.0)}

[lionel-]

So the only meaningful action to take is to fill in default values if and only if the required aesthetics are missing.

I'm not sure I understand the context fully, but maybe it would help if ggplot2 provided descriptor functions for the current value of the aesthetic. Then you could do something like fill = default_aes("fill", "grey20") in the signature. This would make it less magical and obscure.

Another possibly useful way would be to wrap the default value in a sentinel wrapper, so that ggplot2 knows this value should only be set if missing.

[yutannihilation]

@clauswilke
In your case, if you can know all possible variable names in advance, I think data masks can be created with default values.

library(rlang)

eval_with_defaults <- function(q, data) {
  # default values
  mask <- list(x = 0, y = 0)
  # overwrite values if exists
  mask[names(data)] <- data
  
  eval_tidy(q, mask)
}

q <- quo(x + y %||% 10)

eval_with_defaults(q, data.frame(x = 1, y = 1))
#> [1] 2
eval_with_defaults(q, data.frame(x = 1))
#> [1] 1

^{Created on 2020-01-15 by the reprex package (v0.3.0)}

But, it's not the case with facets, IIUC. I thought we need to extract symbols to what variables we need to set active bindings. Sorry, I don't understand your suggestion, @lionel-...

With an "undefined" active binding, this would have the same consequence as normal evaluation:

What is "undefined" active binding? I expected something like this:

eval_with_typed_error <- function(quo, data) {
  possible_variables <- extract_symbols_as_strings(quo)
  local_bindings(!!possible_variables := function(e) raiseNotFoundError())
  eval_tidy(quo, data)
}

[yutannihilation]

restarting interrupted promise evaluation

This comes from here

https://github.com/wch/r-source/blob/3cb780c3f766316a8f75bd6d890720d630ad3e37/src/main/eval.c#L505

This probably indicates some default argument is referenced multiple times? I'm not sure if it's OK or not, though.

[clauswilke]

The warning arises when you try to evaluate an argument, it errors out, and then you evaluate it again. See this SO question:
https://stackoverflow.com/questions/20596902/r-avoiding-restarting-interrupted-promise-evaluation-warning

There's also an answer that shows how to avoid the warning, but I don't understand why it works (and it's not explained).