Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Facetting with complex expressions does not skip layers with missing data #2689

Closed
mundl opened this issue Jun 7, 2018 · 10 comments
Closed
Assignees
Labels
bug an unexpected problem or unintended behavior facets 💎

Comments

@mundl
Copy link

mundl commented Jun 7, 2018

library(ggplot2)
set.seed(2804)
point <- data.frame(x = rnorm(n = 10),
                    y = rnorm(n = 10),
                    class = sample(x = 2, size = 10, replace = TRUE))

poly <- data.frame(x = rnorm(n = 4),
                   y = rnorm(n = 4))

Works as expected, points and polygons (same across facets) are drawn.

ggplot(point, aes(x = x, y = y)) +
    geom_point() +
    geom_polygon(data = poly, fill = NA, col = 1) +
    facet_wrap(vars(class))

Using any function inside vars() or simply vars((class)) gives an error.

ggplot(point, aes(x = x, y = y)) +                                                               
    geom_point() +                                                                                   
    geom_polygon(data = poly, fill = NA, col = 1) +                                                  
    facet_wrap(vars(as.character(class)))        
                                                    
#> Error in as.character(class): cannot coerce type 'builtin' to vector of type 'character'

Using the same code from above but not specifying a data argument in geom_polygon() works.

ggplot(point, aes(x = x, y = y)) +
    geom_point() +
    geom_polygon(fill = NA, col = 1) +
    facet_wrap(vars(as.character(class)))

Session info
devtools::session_info()
#> Session info -------------------------------------------------------------
#>  setting  value                       
#>  version  R version 3.5.0 (2018-04-23)
#>  system   x86_64, mingw32             
#>  ui       RTerm                       
#>  language (EN)                        
#>  collate  German_Austria.1252         
#>  tz       Europe/Berlin               
#>  date     2018-06-07
#> Packages -----------------------------------------------------------------
#>  package    * version    date       source                            
#>  assertthat   0.2.0      2017-04-11 CRAN (R 3.5.0)                    
#>  backports    1.1.2      2017-12-13 CRAN (R 3.5.0)                    
#>  base       * 3.5.0      2018-04-23 local                             
#>  bindr        0.1.1      2018-03-13 CRAN (R 3.5.0)                    
#>  bindrcpp     0.2.2      2018-03-29 CRAN (R 3.5.0)                    
#>  colorspace   1.3-2      2016-12-14 CRAN (R 3.5.0)                    
#>  compiler     3.5.0      2018-04-23 local                             
#>  curl         3.2        2018-03-28 CRAN (R 3.5.0)                    
#>  datasets   * 3.5.0      2018-04-23 local                             
#>  devtools     1.13.5     2018-02-18 CRAN (R 3.5.0)                    
#>  digest       0.6.15     2018-01-28 CRAN (R 3.5.0)                    
#>  dplyr        0.7.4      2017-09-28 CRAN (R 3.5.0)                    
#>  evaluate     0.10.1     2017-06-24 CRAN (R 3.5.0)                    
#>  ggplot2    * 2.2.1.9000 2018-06-07 Github (tidyverse/ggplot2@4db5122)
#>  glue         1.2.0      2017-10-29 CRAN (R 3.5.0)                    
#>  graphics   * 3.5.0      2018-04-23 local                             
#>  grDevices  * 3.5.0      2018-04-23 local                             
#>  grid         3.5.0      2018-04-23 local                             
#>  gtable       0.2.0      2016-02-26 CRAN (R 3.5.0)                    
#>  htmltools    0.3.6      2017-04-28 CRAN (R 3.5.0)                    
#>  httr         1.3.1      2017-08-20 CRAN (R 3.5.0)                    
#>  knitr        1.20       2018-02-20 CRAN (R 3.5.0)                    
#>  labeling     0.3        2014-08-23 CRAN (R 3.5.0)                    
#>  lazyeval     0.2.1      2017-10-29 CRAN (R 3.5.0)                    
#>  magrittr     1.5        2014-11-22 CRAN (R 3.5.0)                    
#>  memoise      1.1.0      2017-04-21 CRAN (R 3.5.0)                    
#>  methods    * 3.5.0      2018-04-23 local                             
#>  mime         0.5        2016-07-07 CRAN (R 3.5.0)                    
#>  munsell      0.4.3      2016-02-13 CRAN (R 3.5.0)                    
#>  pillar       1.2.2      2018-04-26 CRAN (R 3.5.0)                    
#>  pkgconfig    2.0.1      2017-03-21 CRAN (R 3.5.0)                    
#>  plyr         1.8.4      2016-06-08 CRAN (R 3.5.0)                    
#>  R6           2.2.2      2017-06-17 CRAN (R 3.5.0)                    
#>  Rcpp         0.12.16    2018-03-13 CRAN (R 3.5.0)                    
#>  rlang        0.2.1      2018-05-30 CRAN (R 3.5.0)                    
#>  rmarkdown    1.9        2018-03-01 CRAN (R 3.5.0)                    
#>  rprojroot    1.3-2      2018-01-03 CRAN (R 3.5.0)                    
#>  scales       0.5.0      2017-08-24 CRAN (R 3.5.0)                    
#>  stats      * 3.5.0      2018-04-23 local                             
#>  stringi      1.1.7      2018-03-12 CRAN (R 3.5.0)                    
#>  stringr      1.3.0      2018-02-19 CRAN (R 3.5.0)                    
#>  tibble       1.4.2      2018-01-22 CRAN (R 3.5.0)                    
#>  tools        3.5.0      2018-04-23 local                             
#>  utils      * 3.5.0      2018-04-23 local                             
#>  withr        2.1.2      2018-03-15 CRAN (R 3.5.0)                    
#>  xml2         1.2.0      2018-01-24 CRAN (R 3.5.0)                    
#>  yaml         2.1.19     2018-05-01 CRAN (R 3.5.0)
@hadley hadley added the bug an unexpected problem or unintended behavior label Jun 7, 2018
@hadley hadley added this to the v2.3.0 milestone Jun 7, 2018
@hadley
Copy link
Member

hadley commented Jun 7, 2018

@lionel- can you take a look at this one too please?

@lionel- lionel- self-assigned this Jun 7, 2018
@lionel-
Copy link
Member

lionel- commented Jun 11, 2018

@hadley What are the semantics for layers with data? should we have the ggplot data as an intermediate mask, so that class (from the tibble passed to ggplot()) would be available in the layer evaluation in all circumstances?

@hadley
Copy link
Member

hadley commented Jun 12, 2018

Hmmm, I think that's right — although to preserve the existing semantics that environment would binding every existing variable to NULL

@lionel-
Copy link
Member

lionel- commented Jun 18, 2018

@mundl Are you sure this used to work? I get the same results with CRAN ggplot2.

@hadley We want the same evaluation semantics as in the last version right?

It doesn't seem merging the layer data frames would be much of an improvement since it only makes sense in the particular case where they have the same number of rows.

@lionel-
Copy link
Member

lionel- commented Jun 18, 2018

Ah I get it now, this is about ignoring facet specifications that refer to variables that do not exist in the current layer.

I think we can pull this off by defining active bindings for all variables in the ggplot() layer that are missing in the current layer. These active bindings would send a condition that'd be picked up by the facet evaluation, signalling that facetting should be skipped for this layer.

Only downside: this prevents referring to variables from the environment. But you'll still be able to unquote those.

@hadley
Copy link
Member

hadley commented Jun 18, 2018

I think if they’re bound to NULL that will achieve the same effect. But if this isn’t a regression, there’s no need to work on now.

@lionel-
Copy link
Member

lionel- commented Jun 18, 2018

Simple variables already work, this is about giving complex expressions the same semantics.

@lionel- lionel- changed the title Facetting doesn't work using expressions inside vars() Facetting with complex expressions does not skip layers with missing data Jun 18, 2018
@lionel- lionel- removed this from the v2.3.0 milestone Jun 18, 2018
@lionel-
Copy link
Member

lionel- commented Jun 18, 2018

When we fix this it'd be a good time to refactor the facetting code so the variables are evaluated only once. In the example above I see 5 evaluations of the vars() expression.

@paleolimbot
Copy link
Member

The vetoed portion of the PR for #2693 may help with this...I put it in a gist as it didn't end up in the PR:

https://gist.github.com/paleolimbot/832d30625d2cd46f39086228355a9dc0

@paleolimbot
Copy link
Member

This was fixed in #3757!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug an unexpected problem or unintended behavior facets 💎
Projects
None yet
Development

No branches or pull requests

4 participants