Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Reliance on positional matching when also passing ... is error prone #713

Closed
BrianDiggs opened this Issue · 2 comments

2 participants

Brian Diggs Hadley Wickham
Brian Diggs

When passing specifying certain arguments to functions and passing the rest from the calling function via ..., reliance on positional matching of arguments rather than using named arguments can lead to the wrong arguments being assigned to the wrong formals.

This issue came to light in a discussion about the palette argument to discrete_scale which is not available to, for example, scale_shape_discrete (see https://groups.google.com/d/topic/ggplot2/XeC40tWpIPw/discussion).

Using scale_shape as an example because it is simple:

scale_shape <- function(..., solid = TRUE) {
  discrete_scale("shape", "shape_d", shape_pal(solid), ...)
}

The first three arguments to discrete_scale are aesthetics, scale_name, and palette. If an argument is specified with any of these names in the scale_shape call, it will preferentially be matched to the named argument (according to the rules of argument matching). The given unnamed arguments are then assigned positionally to the remaining arguments and so do not get to the right place. This was the problem in the mailing list discussion referenced above.

Note especially the value of the name element:

> p.test <- function(n) { 21:(20+n) }
> dput(scale_shape(palette=p.test))
structure(list(call = discrete_scale(aesthetics = "shape", scale_name = "shape_d", 
    palette = ..1, name = shape_pal(solid)), aesthetics = "shape", 
    scale_name = "shape_d", palette = function (n) 
    {
        21:(20 + n)
    }, range = <S4 object of class structure("DiscreteRange", package = "scales")>, 
    limits = NULL, na.value = NA, expand = structure(list(), class = "waiver"), 
    name = function (n) 
    {
        if (n > 6) {
            msg <- paste("The shape palette can deal with a maximum of 6 discrete ", 
                "values because more than 6 becomes difficult to discriminate; ", 
                "you have ", n, ". Consider specifying shapes manually. if you ", 
                "must have them.", sep = "")
            warning(paste(strwrap(msg), collapse = "\n"), call. = FALSE)
        }
        if (solid) {
            c(16, 17, 15, 3, 7, 8)[seq_len(n)]
        }
        else {
            c(1, 2, 0, 3, 7, 8)[seq_len(n)]
        }
    }, breaks = structure(list(), class = "waiver"), labels = structure(list(), class = "waiver"), 
    legend = NULL, drop = TRUE, guide = "legend"), .Names = c("call", 
"aesthetics", "scale_name", "palette", "range", "limits", "na.value", 
"expand", "name", "breaks", "labels", "legend", "drop", "guide"
), class = c("shape_d", "discrete", "scale"))
> str(scale_shape(palette=p.test))
List of 14
 $ call      : language discrete_scale(aesthetics = "shape", scale_name = "shape_d", palette = ..1,      name = shape_pal(solid))
 $ aesthetics: chr "shape"
 $ scale_name: chr "shape_d"
 $ palette   :function (n)  
  ..- attr(*, "srcref")=Class 'srcref'  atomic [1:8] 1 11 1 35 11 35 1 1
  .. .. ..- attr(*, "srcfile")=Classes 'srcfilecopy', 'srcfile' <environment: 0x000000001f0dabc0> 
 $ range     :Reference class 'DiscreteRange' [package "scales"] with 1 fields
  ..$ range: NULL
  ..and 14 methods, of which 3 are possibly relevant:
  ..  initialize, reset, train
 $ limits    : NULL
 $ na.value  : logi NA
 $ expand    : list()
  ..- attr(*, "class")= chr "waiver"
 $ name      :function (n)  
 $ breaks    : list()
  ..- attr(*, "class")= chr "waiver"
 $ labels    : list()
  ..- attr(*, "class")= chr "waiver"
 $ legend    : NULL
 $ drop      : logi TRUE
 $ guide     : chr "legend"
 - attr(*, "class")= chr [1:3] "shape_d" "discrete" "scale"

If scale_shape was instead defined as

scale_shape <- function(..., solid = TRUE) {
  discrete_scale(aesthetics = "shape", scale_name = "shape_d", palette = shape_pal(solid), ...)
}

Then at least specifying palette in the scale_shape call would fail sooner with:

p.test <- function(n) { 21:(20+n) }
> scale_shape(palette=p.test)
Error in discrete_scale(aesthetics = "shape", scale_name = "shape_d",  : 
  formal argument "palette" matched by multiple actual arguments

Ideally, the ... argument would be parsed and any disallowed values would be ignored or warned about. Perhaps something like

scale_shape <- function(..., solid = TRUE) {
  dots <- list(...)
  dots[["aesthetics"]] <- "shape"
  dots[["scale_name"]] <- "shape_d"
  dots[["palette"]] <- shape_pal(solid)
  do.call("discrete_scale", dots)
}

All instances that pass ... on to other functions and rely on positional matching of arguments need to be assessed.

Hadley Wickham
Owner

I think the best solution is to explicitly name all the arguments discrete_scale etc. I rather fix the problem with standard R approaches, rather than having to do something special.

Hadley Wickham
Owner

This sounds like a great feature, but unfortunately we don't currently have the development bandwidth to support it. If you'd like to submit a pull request that implements this feature, please follow the instructions in the development vignette.

Hadley Wickham hadley closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.