Add error messages when using + with layers and scales

[jrnold]

In my experience, one of the most common and inscrutable error messages for new users is when they forget a + and get the error message: non-numeric argument to binary operator. For example, something like this:

> ggplot(mtcars, aes(x = disp, y = mpg))
> geom_point() + geom_smooth()
Error in geom_point() + geom_smooth() : 
  non-numeric argument to binary operator

I would like to add a more informative error message in the style of #2056 and the error messages in dplyr, e.g. this.

> ggplot(mtcars, aes(x = disp, y = mpg))
> geom_point() + geom_smooth()
Error in `+.gg`(scale_fill_brewer(), geom_point()) : 
  Cannot add ggproto objects together. Did you forget to add this object to a ggplot() object?

The obvious approach would be to add a function(s) +.Layer or +.ggproto, but when there is a valid addition of a ggplot and Layer object that produced an "Incompatible Methods" error since S3 generics in the Ops group dispatch on both arguments and only work if only one has a specific method, or both have the same method. Something which it seems like you encountered before here.

So the only way I could make this work is to make ggplot, theme and ggproto inherit from the same S3 class, and define the + method with that class. As far as I can tell, currently the gg class is used for this purpose with respect to the ggplot and theme classes, with only the +.gg method defined. My solution was to change the class given to default ggproto() objects to c("ggproto", "gg"). This feels unclean, but didn't break any of the tests. But, it seemed like the simplest way accomplish the stated objective. Given that this is adding a class to ggproto objects, it may not be worth it to change the error messages, but I thought I'd propose the change.

[hadley]

I think this is a reasonable thing to do.

Can you please make this one small change and add a bullet to NEWS?

[hadley]

Needs call. = FALSE

[jrnold]

I also realized that this change could cause problems for anyone who has written code that tests for class the wrong way, class(x) == "ggproto".

[hadley]

Thanks!

I think it's ok to break code that doesn't use inherits()

[lock]

This old issue has been automatically locked. If you believe you have found a related problem, please file a new issue (with reprex) and link to this issue. https://reprex.tidyverse.org/