teunbrand / ggh4x

ggplot extension: options for tailored facets, multiple colourscales and miscellaneous
https://teunbrand.github.io/ggh4x/
Other
534 stars 32 forks source link

`facet_nested()` doesn't work with factors if a layer doens't inclu #117

Closed eliocamp closed 11 months ago

eliocamp commented 11 months ago

When the faceting variable is a factor and not all layers have the faceting variable facet_nested() throws error

Error in `vec_rbind()`:
! Can't combine `..1$cyl` <factor<01bed>> and `..2$cyl` <double>.

As an example. This works:

library(ggplot2)

base <- ggplot(mapping = aes(mpg, disp)) +
  geom_point(data = subset(mtcars, cyl != 4)) +
  geom_point(data = subset(mtcars, cyl == 4) |> 
               transform(cyl = NULL), 
             color = "gray") 

base +
  ggh4x::facet_nested(am ~ cyl + vs)

But if cyl is a factor, it doesn't.

mtcars$cyl <- factor(mtcars$cyl)

base %+% mtcars + 
  ggh4x::facet_nested(am ~ cyl + vs)
#> Error in `vec_rbind()`:
#> ! Can't combine `..1$cyl` <factor<01bed>> and `..2$cyl` <double>.
#> Backtrace:
#>      ▆
#>   1. ├─base::tryCatch(...)
#>   2. │ └─base (local) tryCatchList(expr, classes, parentenv, handlers)
#>   3. │   ├─base (local) tryCatchOne(...)
#>   4. │   │ └─base (local) doTryCatch(return(expr), name, parentenv, handler)
#>   5. │   └─base (local) tryCatchList(expr, names[-nh], parentenv, handlers[-nh])
#>   6. │     └─base (local) tryCatchOne(expr, names, parentenv, handlers[[1L]])
#>   7. │       └─base (local) doTryCatch(return(expr), name, parentenv, handler)
#>   8. ├─base::withCallingHandlers(...)
#>   9. ├─base::saveRDS(...)
#>  10. ├─base::do.call(...)
#>  11. ├─base (local) `<fn>`(...)
#>  12. ├─global `<fn>`(input = base::quote("beton-fish_reprex.R"))
#>  13. │ └─rmarkdown::render(input, quiet = TRUE, envir = globalenv(), encoding = "UTF-8")
#>  14. │   └─knitr::knit(knit_input, knit_output, envir = envir, quiet = quiet)
#>  15. │     └─knitr:::process_file(text, output)
#>  16. │       ├─base::withCallingHandlers(...)
#>  17. │       ├─base::withCallingHandlers(...)
#>  18. │       ├─knitr:::process_group(group)
#>  19. │       └─knitr:::process_group.block(group)
#>  20. │         └─knitr:::call_block(x)
#>  21. │           └─knitr:::block_exec(params)
#>  22. │             └─knitr:::eng_r(options)
#>  23. │               ├─knitr:::in_input_dir(...)
#>  24. │               │ └─knitr:::in_dir(input_dir(), expr)
#>  25. │               └─knitr (local) evaluate(...)
#>  26. │                 └─evaluate::evaluate(...)
#>  27. │                   └─evaluate:::evaluate_call(...)
#>  28. │                     ├─evaluate (local) handle(...)
#>  29. │                     │ └─base::try(f, silent = TRUE)
#>  30. │                     │   └─base::tryCatch(...)
#>  31. │                     │     └─base (local) tryCatchList(expr, classes, parentenv, handlers)
#>  32. │                     │       └─base (local) tryCatchOne(expr, names, parentenv, handlers[[1L]])
#>  33. │                     │         └─base (local) doTryCatch(return(expr), name, parentenv, handler)
#>  34. │                     ├─base::withCallingHandlers(...)
#>  35. │                     ├─base::withVisible(value_fun(ev$value, ev$visible))
#>  36. │                     └─knitr (local) value_fun(ev$value, ev$visible)
#>  37. │                       └─knitr (local) fun(x, options = options)
#>  38. │                         ├─base::withVisible(knit_print(x, ...))
#>  39. │                         ├─knitr::knit_print(x, ...)
#>  40. │                         └─knitr:::knit_print.default(x, ...)
#>  41. │                           └─evaluate (local) normal_print(x)
#>  42. │                             ├─base::print(x)
#>  43. │                             └─ggplot2:::print.ggplot(x)
#>  44. │                               ├─ggplot2::ggplot_build(x)
#>  45. │                               └─ggplot2:::ggplot_build.ggplot(x)
#>  46. │                                 └─layout$setup(data, plot$data, plot$plot_env)
#>  47. │                                   └─ggplot2 (local) setup(..., self = self)
#>  48. │                                     └─self$facet$compute_layout(data, self$facet_params)
#>  49. │                                       └─ggh4x (local) compute_layout(..., self = self)
#>  50. │                                         └─self$vars_combine(data, params$plot_env, cols, drop = params$drop)
#>  51. │                                           └─ggh4x (local) vars_combine(...)
#>  52. │                                             ├─ggh4x:::unique0(vec_rbind(!!!values[has_all]))
#>  53. │                                             └─vctrs::vec_rbind(!!!values[has_all])
#>  54. └─vctrs (local) `<fn>`()
#>  55.   └─vctrs::vec_default_ptype2(...)
#>  56.     ├─base::withRestarts(...)
#>  57.     │ └─base (local) withOneRestart(expr, restarts[[1L]])
#>  58.     │   └─base (local) doWithOneRestart(return(expr), restart)
#>  59.     └─vctrs::stop_incompatible_type(...)
#>  60.       └─vctrs:::stop_incompatible(...)
#>  61.         └─vctrs:::stop_vctrs(...)
#>  62.           └─rlang::abort(message, class = c(class, "vctrs_error"), ..., call = call)

The error goes away if I remove the second layer.

ggplot(mapping = aes(mpg, disp)) +
  geom_point(data =  subset(mtcars, cyl != 4)) +
  ggh4x::facet_nested(am ~ cyl + vs)

Created on 2023-07-19 with reprex v2.0.2

teunbrand commented 11 months ago

Yeah I think ggplot2 has moved ahead on this with a smarter warning/rescue while facet_nested() uses an older approach.

library(ggplot2)

base <- ggplot(mapping = aes(mpg, disp)) +
  geom_point(data = subset(mtcars, cyl != 4)) +
  geom_point(data = subset(mtcars, cyl == 4) |> 
               transform(cyl = NULL), 
             color = "gray") 

mtcars$cyl <- factor(mtcars$cyl)

base %+% mtcars +
  facet_grid(am ~ cyl + vs)
#> Warning: Combining variables of class <factor> and <numeric> was deprecated in ggplot2
#> 3.4.0.
#> ℹ Please ensure your variables are compatible before plotting (location:
#>   `combine_vars()`)
#> This warning is displayed once every 8 hours.
#> Call `lifecycle::last_lifecycle_warnings()` to see where this warning was
#> generated.
#> Warning: Combining variables of class <numeric> and <factor> was deprecated in ggplot2
#> 3.4.0.
#> ℹ Please ensure your variables are compatible before plotting (location:
#>   `combine_vars()`)
#> This warning is displayed once every 8 hours.
#> Call `lifecycle::last_lifecycle_warnings()` to see where this warning was
#> generated.

Created on 2023-07-19 with reprex v2.0.2

eliocamp commented 11 months ago

What I don't understand is why is ggplot2 combining variables of different type. 🤔

teunbrand commented 11 months ago

IIRC they switched a bunch of internals to {vctrs} and came across some cases where {vctrs} was too strict (for backward compatibility) about combining different types of variables and then resolved that by being more permissive. The secret sauce is in ggplot2:::vec_rbind0(), but this reaches very deep into the internals with weird restart invocations. I had to work with this function at some point and then and there decided that I'm never going to copy those internals due to their depth and niche functionality.

teunbrand commented 11 months ago

I'm going to close this issue as this is some backward compatibility issue that ggplot2 solved, but I'm not going to commit to fully mirror all solutions ggplot2 has in place for backward compatibility.