teunbrand / ggh4x

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

Persisting error in facet_nested #49

Closed ayeletperes closed 2 years ago

ayeletperes commented 2 years ago

Hey,

I've tried using the function facet_nested but I get this error: "Error: widths must be a unit object"

I thought it might be my code, but it also happens with the example code in the README.

Session info:

image

Thanks

teunbrand commented 2 years ago

Hi there,

Yes I can reproduce this bug on R3.6.3 and I think it is due to pre-R4.0 unit operations being more fragile than >R4.0.

For example, on R3.6.3:

(x <- grid::unit(c(1, 2), "npc"))
#> [1] 1npc 2npc

x[[1]]
#> [1] 1

Whereas on R4.0.5:

(x <- grid::unit(c(1, 2), "npc"))
#> [1] 1npc 2npc

x[[1]]
#> [1] 1npc

Perhaps I should set the R versions requirement to >4.0.0 in the description file. I'll leave this open in case other people also experience this bug. I think the previous version didn't have this bug in R3.6, so if you specifically want to use facet_nested, you could install that version with devtools::install_version("ggh4x", "0.1.2.1") or update your R version.

mimi3421 commented 2 years ago

I think this situation may be only due to the little difference / bug R treating rep(x, length.out=y) and rep_len(x, length.out=y) in different version. In R 3.6, rep(unit(1,"npc"),length.out=2) returns [1] 1npc 1npc while rep_len(unit(1,"npc"),length.out=2) returns [1] 1 1.

I change all the rep_len(...) function to rep(x, length.out) and it seems that the compatibility problem solved in R 3.6, like the rep_len in Facet_wrap2.R > FacetWrap2 > setup_panel_table

setup_panel_table = function(self, panels, layout, #empty, position,
                               theme, coord, ranges, params) {
    ncol <- params$dim[2] %||% max(c(layout$.LEFT, layout$.RIGHT))
    nrow <- params$dim[1] %||% max(c(layout$.TOP,  layout$.BOTTOM))

    aspect <- self$setup_aspect_ratio(coord, params$free, theme, ranges)

    respect <- params$respect %||% attr(aspect, "respect") %||% FALSE
    widths  <- params$widths  %||% unit(1, "null")
    heights <- params$heights %||% unit(abs(aspect %||% 1), "null")

    panel_table <- gtable(
      #widths  = rep_len(widths, ncol),
      #heights = rep_len(heights, nrow),
      widths  = rep(widths, length.out = ncol),
      heights = rep(heights, length.out = nrow),
      respect = respect
    )
    panel_table <- gtable_add_grob(
      panel_table, panels,
      t = layout$.TOP,  b = layout$.BOTTOM,
      l = layout$.LEFT, r = layout$.RIGHT,
      z = 1, clip = coord$clip, name = paste0("panel-", seq_along(panels))
    )
    panel_table <- gtable_add_col_space(
      panel_table, calc_element("panel.spacing.x", theme)
    )
    panel_table <- gtable_add_row_space(
      panel_table, calc_element("panel.spacing.y", theme)
    )
    panel_table
  },
teunbrand commented 2 years ago

I can replace rep_len(x, y) with rep(x, length.out = y) without breaking any unit tests on R4.0, if that helps for people working in R 3.6. The unit tests on R3.6 will remain broken* due to the update to grid units, so I can't really promise everything is fixed, but it seems possible to use nested facets again. Let me know if that sounds agreeable to you.

* Checking for correctness of grid::unit() output is different between the two versions and I'm not keen on refactoring unit tests with branches for different R versions.

mimi3421 commented 2 years ago

Thanks for maintaining this excellent package.

I aggree with you. I also consider it would be a hard work to maintain two versions at the same time as great changes has been made between the two version of R.

I have uploaded a simply fixed version https://github.com/mimi3421/ggh4x/releases/tag/v0.2.0_fixed_for_R3.6 if anyone wants to have a test.

teunbrand commented 2 years ago

Thanks mimi3421 for having uploaded a tarball with a fixed version. For future cases, feel free to open a pull request with suggested changes. It's a bit more transparent to people what code has been changed and github will show that you've contributed.

Meanwhile, I also have commited some changes that I think might fix this problem for R3.6. These can be installed with:

remotes::install_github("teunbrand/ggh4x@4e242295da442e0e412c7c3e713a383f603ceda0")
llrs commented 2 years ago

Many thanks for providing this hot fix (and the package!). I can confirm 4e242295da442e0e412c7c3e713a383f603ceda0 works for me on R 3.6.3.

teunbrand commented 2 years ago

The fix is soon propagated to CRAN, so I'll close this.