trevorld / ggpattern

ggplot geoms with pattern fills
https://trevorldavis.com/R/ggpattern/dev/
Other
361 stars 18 forks source link

[BUG] geom_col_pattern switching places unintendedly when position="dodge" #97

Closed matanhakim closed 1 year ago

matanhakim commented 1 year ago

Hi, thanks a lot for this package! This is my first time issuing a bug reprot, so please inform me if anything is missing.

Bug description

I'm using ggpattern and having a weird bug: in some plots, two geom_col_pattern are switching places. This happens only in some plots, and only in one place in each of these bad plots. when I run the same code but with geom_col rather than geom_col_pattern there is no problem. Watch for the lowest-left pair of columns - in the first plot they are wrongly switched, and in the second plot they are fine.

Minimal, reproducible example

library(tidyverse)
#> Warning: package 'tidyverse' was built under R version 4.2.2
#> Warning: package 'tidyr' was built under R version 4.2.2
#> Warning: package 'readr' was built under R version 4.2.2
#> Warning: package 'purrr' was built under R version 4.2.2
#> Warning: package 'dplyr' was built under R version 4.2.2
#> Warning: package 'stringr' was built under R version 4.2.2
#> Warning: package 'forcats' was built under R version 4.2.2
#> Warning: package 'lubridate' was built under R version 4.2.2
library(ggpattern)
#> Warning: package 'ggpattern' was built under R version 4.2.2
library(scales)
#> 
#> Attaching package: 'scales'
#> The following object is masked from 'package:purrr':
#> 
#>     discard
#> The following object is masked from 'package:readr':
#> 
#>     col_factor

df_bug <- read_csv("https://raw.githubusercontent.com/matanhakim/general_files/main/bug_geom_col_pattern.csv") %>% 
  mutate(year = factor(year))
#> Rows: 30 Columns: 7
#> ── Column specification ────────────────────────────────────────────────────────
#> Delimiter: ","
#> chr (3): reason_5, gil_3, rse_sig
#> dbl (4): year, n, pct, rse
#> 
#> ℹ Use `spec()` to retrieve the full column specification for this data.
#> ℹ Specify the column types or set `show_col_types = FALSE` to quiet this message.

df_bug %>% 
  ggplot(aes(reason_5, pct, fill = year)) +
    geom_col_pattern(aes(pattern_density = rse_sig), position = "dodge", pattern = "stripe") +
    geom_text(aes(label = paste0(round(pct * 100, digits = 1), "%")), vjust = -0.5, position = position_dodge(width = 1)) +
    scale_y_continuous(labels = label_percent(), expand = expansion(mult = c(0, 0.2))) +
    facet_wrap(vars(gil_3), scales = "free_x", ncol = 1) +
    scale_pattern_density_manual(
      values = c("high" = 0, "medium" = 0.25, "low" = 0.5),
      breaks = c("medium", "low")
    ) +
    guides(
      fill = guide_legend(override.aes = list(pattern = "none")),
      pattern_density = guide_legend(override.aes = list(fill = "white", color = "black"))
    )


df_bug %>% 
  ggplot(aes(reason_5, pct, fill = year)) +
  geom_col(position = "dodge") +
  geom_text(aes(label = paste0(round(pct * 100, digits = 1), "%")), vjust = -0.5, position = position_dodge(width = 1)) +
  scale_y_continuous(labels = label_percent(), expand = expansion(mult = c(0, 0.2))) +
  facet_wrap(vars(gil_3), scales = "free_x", ncol = 1)

Created on 2023-03-04 with reprex v2.0.2

Session info

Please enter here the results of xfun::session_info("ggpattern")

> xfun::session_info("ggpattern")
R version 4.2.1 (2022-06-23 ucrt)
Platform: x86_64-w64-mingw32/x64 (64-bit)
Running under: Windows 10 x64 (build 19045), RStudio 2022.12.0.353

Locale:
  LC_COLLATE=English_United States.utf8  LC_CTYPE=English_United States.utf8   
  LC_MONETARY=English_United States.utf8 LC_NUMERIC=C                          
  LC_TIME=English_United States.utf8    

Package version:
  cachem_1.0.6       class_7.3.20       classInt_0.4.8     cli_3.6.0         
  colorspace_2.0.3   DBI_1.1.3          e1071_1.7.12       fansi_1.0.3       
  farver_2.1.1       fastmap_1.1.0      ggpattern_1.0.1    ggplot2_3.4.1     
  glue_1.6.2         graphics_4.2.1     grDevices_4.2.1    grid_4.2.1        
  gridpattern_1.0.2  gtable_0.3.1       isoband_0.2.6      KernSmooth_2.23.20
  labeling_0.4.2     lattice_0.20.45    lifecycle_1.0.3    magrittr_2.0.3    
  MASS_7.3.58.1      Matrix_1.4.1       memoise_2.0.1      methods_4.2.1     
  mgcv_1.8.40        munsell_0.5.0      nlme_3.1.160       pillar_1.8.1      
  pkgconfig_2.0.3    png_0.1.7          proxy_0.4.27       R6_2.5.1          
  RColorBrewer_1.1.3 Rcpp_1.0.9         rlang_1.0.6        s2_1.1.2          
  scales_1.2.1       sf_1.0.9           splines_4.2.1      stats_4.2.1       
  tibble_3.1.8       tools_4.2.1        units_0.8.1        utf8_1.2.2        
  utils_4.2.1        vctrs_0.5.2        viridisLite_0.4.1  withr_2.5.0       
  wk_0.7.1        
trevorld commented 1 year ago
df_bug %>% 
  ggplot(aes(reason_5, pct, fill = year)) +
  geom_col_pattern(position = "dodge") +
  geom_text(aes(label = paste0(round(pct * 100, digits = 1), "%")), vjust = -0.5, position = position_dodge(width = 1)) +
  scale_y_continuous(labels = label_percent(), expand = expansion(mult = c(0, 0.2))) +
  facet_wrap(vars(gil_3), scales = "free_x", ncol = 1)

image

matanhakim commented 1 year ago

Thanks for the answer! I'm not familiar enough with {ggplot2} structure, how can you tell if it's a problem in {ggplot2} or in {ggpattern}? If you're sure it's in {ggplot2}, I'll file an issue over there. Additionally, now that you mention it, this might have come up after installing the newest version of the {tidyverse}, which possibly resulted in a new version of {ggplot2}.

trevorld commented 1 year ago

how can you tell if it's a problem in {ggplot2} or in {ggpattern}

If you can trigger this column flip purely in {ggplot2} by adding and manipulating another aesthetic in geom_col() (perhaps the border line type, color, width etc.) then you could confirm it lies with {ggplot2} in a reproducible fashion for them to fix.

If you look in the source code of {ggpattern} then geom_col_pattern() calls GeomColPattern calls GeomBarPattern calls GeomRectPattern calls GeomRect {ggproto} objects. Nowhere do {ggpattern} create and use its own "Position" {ggproto} objects or calculate for itself where the polygons we are going to be filling are placed in the charts. For example if you search the code for "dodge" you'll observe that there are zero "dodge" calculations being made in {ggpattern}.

But perhaps there is a subtle difference in how {ggplot2} currently constructs GeomCol and how {ggpattern} constructs GeomColPattern that was introduced after @coolbutuseless originally forked {ggplot2} a couple of years ago to make {ggpattern} and this subtle difference in parameters makes a difference in this one edge case... I guess you'd need to be keeping a very close track of all the low-level commits being made upstream in {ggplot2}. The {ggpattern} maintainers have not been doing this...

matanhakim commented 1 year ago

You are correct - the bug is in {ggplot2} itself. I've filed an issue for {ggplot2} here, you can see the reprex and how it's using only {ggplot2}. Thanks a lot for the help, this issue can be closed.