From 835cf08e400ce60e05c04c50f9d8ce9d5102b51f Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Fri, 14 Jun 2024 11:46:24 +0200 Subject: [PATCH 1/3] evaluate facets before adding margins --- R/facet-grid-.R | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/R/facet-grid-.R b/R/facet-grid-.R index 0854b5299b..7dc9beb971 100644 --- a/R/facet-grid-.R +++ b/R/facet-grid-.R @@ -297,12 +297,21 @@ FacetGrid <- ggproto("FacetGrid", Facet, return(data) } - # Compute faceting values and add margins - margin_vars <- list(intersect(names(rows), names(data)), - intersect(names(cols), names(data))) - data <- reshape_add_margins(data, margin_vars, params$margins) - + # Compute faceting values facet_vals <- eval_facets(c(rows, cols), data, params$.possible_columns) + if (nrow(facet_vals) == nrow(data)) { + # Margins are computed on evaluated faceting values (#1864). + facet_vals <- reshape_add_margins( + # We add an index column to track data recycling + vec_cbind(facet_vals, .index = seq_len(nrow(facet_vals))), + list(intersect(names(rows), names(facet_vals)), + intersect(names(cols), names(facet_vals))), + params$margins + ) + # Apply recycling on original data to fit margins + data <- vec_slice(data, facet_vals$.index) + facet_vals$.index <- NULL + } # If any faceting variables are missing, add them in by # duplicating the data From 42e10e9ac2f5a275e125b4ab85d8b22ac39e76f5 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Fri, 14 Jun 2024 11:46:29 +0200 Subject: [PATCH 2/3] add test --- tests/testthat/test-facet-map.R | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/testthat/test-facet-map.R b/tests/testthat/test-facet-map.R index 35e3256958..de2bf20af2 100644 --- a/tests/testthat/test-facet-map.R +++ b/tests/testthat/test-facet-map.R @@ -22,6 +22,10 @@ test_that("margins add extra data", { loc <- panel_map_one(facet_grid(a~b, margins = "b"), df) expect_equal(nrow(loc), nrow(df) * 2) + + # For variables including computation (#1864) + loc <- panel_map_one(facet_grid(a ~ I(b + 1), margins = TRUE), df) + expect_equal(nrow(loc), nrow(df) * 4) }) test_that("grid: missing facet columns are duplicated", { From 23e2c116fe94562c78bb2d867d70efb8cdd49e18 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Fri, 14 Jun 2024 11:48:31 +0200 Subject: [PATCH 3/3] add news bullet --- NEWS.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS.md b/NEWS.md index 67c07b0b05..10eea3f3c2 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,7 @@ # ggplot2 (development version) +* Fixed bug in `facet_grid(margins = TRUE)` when using expresssions + (@teunbrand, #1864). * `geom_rug()` prints a warning when `na.rm = FALSE`, as per documentation (@pn317, #5905) * `position_dodge(preserve = "single")` now handles multi-row geoms better, such as `geom_violin()` (@teunbrand based on @clauswilke's work, #2801).