Checking `...`

[ColinFay]

Following this twitter thread.

labs() uses ..., but the name of the arguments are not checked, so this can lead to errors when passing tile = instead of title, or any other typo.

It would be nice for the user to have at least a warning (or an error) here:

library(ggplot2)
ggplot(iris, aes(Sepal.Length, Sepal.Width)) + 
  geom_point() + 
  labs(tile = "Title has a typo")

[hadley]

And in all other functions that use ...

[yutannihilation]

Does this require us to replace all list() with rlang::list2()? Otherwise, list() actually uses ....

f1 <- function(..., title = "foo") {
  ellipsis::check_dots_used()
  
  list(..., title = title)
}

f2 <- function(..., title = "foo") {
  ellipsis::check_dots_used()
  
  rlang::list2(..., title = title)
}

f1(tile = "Title has a typo")
#> $tile
#> [1] "Title has a typo"
#> 
#> $title
#> [1] "foo"
f2(tile = "Title has a typo")
#> Warning: Some components of ... were not used: tile
#> $tile
#> [1] "Title has a typo"
#> 
#> $title
#> [1] "foo"

^{Created on 2019-03-24 by the reprex package (v0.2.1)}

[yutannihilation]

Hmm, I come to think we can do nothing for labs(); there's a chance tile is a valid aesthetic, and labs() takes arbitrary aesthetics, which means we cannot verify the arguments without annoying false positive warnings. For example:

library(ggplot2)

labs <- function (..., title = waiver(), subtitle = waiver(), caption = waiver(), 
          tag = waiver()) {
  ellipsis::check_dots_used()
  
  args <- rlang::list2(..., title = title, subtitle = subtitle, caption = caption, tag = tag)
  
  is_waive <- vapply(args, ggplot2:::is.waive, logical(1))
  args <- args[!is_waive]
  args <- args[!duplicated(names(args))]
  args <- ggplot2:::rename_aes(args)
  
  structure(args, class = "labels")
}

labs(tile = "Title has a typo")
#> Warning: Some components of ... were not used: tile
#> $tile
#> [1] "Title has a typo"
#> 
#> attr(,"class")
#> [1] "labels"

labs(colour = "colour is a valid aesthetic")
#> Warning: Some components of ... were not used: colour
#> $colour
#> [1] "colour is a valid aesthetic"
#> 
#> attr(,"class")
#> [1] "labels"

^{Created on 2019-03-24 by the reprex package (v0.2.1)}

[thomasp85]

We could do something at build time if we really wants to because all named aesthetics is known there. Not sure it is worth it, though

[yutannihilation]

Thanks. For this particular issue, I agree it's build time if we do some verification.

[yutannihilation]

To clarify, this issue is a general discussion about checking .... It seems we need different approaches for different places. For example:

• use ellipsis package
• check parameters on build time
• modify formals instead of using ... (Let some wrapper functions not to use ... #3210 by @atusy)

[atusy]

At least for scale_x|y_*, modifying formals is a user friendly way.
However, I'm not sure for others.

For example, see scale_alpha_continuous2 implemented by modifying formals of continuous_scale, which works in the same way as original scale_alpha_continuous.
This implementation enables checking arguments on run and suggesting arguments on IDE.
However, first 3 arguments of scale_alpha_continuous2 are aesthetics, scale_name, and palette, which are not intended to be user-specified in most cases.

Thus, I guess reordering of formals may also help users so that aesthetics, scale_name, and palette go to the tail of formals.

In this way, 1st to the 5th arguments of scale_alpha_continuous2 becomes "name", "breaks", "minor_breaks", "labels", and "limits", which are consistent to the order of formals of scale_x|y_continuous.

library(ggplot2)
library(scales)
scale_alpha_continuous2 <- continuous_scale
formals(scale_alpha_continuous2)[c(
  "aesthetics", "scale_name", "palette", "range"
  )] <-
  alist("alpha", "alpha_c", rescale_pal(range), c(0.1, 1))
qplot(x=1:10, y=1:10, alpha = 1:10) +
  scale_alpha_continuous2()

^{Created on 2019-03-29 by the reprex package (v0.2.1)}

[hadley]

I do not think modifying formals is the right way to approach this problem.

[yutannihilation]

I too feel modifying formals might not be the direct answer here, but I think the point is that in some cases we don't really need ... just to create the variations of functions with different defaults. I don't think we have any nice way to do this (especially, in terms of roxygen's support), but, to me, it seems interesting to generate functions programmatically.

[hadley]

A good next step on this issue would be to systematically survey all use of ... and figure out what category they fall into

[paleolimbot]

It's a bit too long for an issue comment, but I put a summary of all 217 uses (excluding s3 generics and methods) here: https://gist.github.com/paleolimbot/708effa39e67bb53f54f30499562587b