From ec262692c334f31330a86f1b3b536219bdc8998f Mon Sep 17 00:00:00 2001 From: Teun van den Brand <49372158+teunbrand@users.noreply.github.com> Date: Wed, 26 Jul 2023 22:15:07 +0200 Subject: [PATCH 1/5] Return NULL when fit fails --- R/stat-smooth.R | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/R/stat-smooth.R b/R/stat-smooth.R index 691d16fa02..0081b380c8 100644 --- a/R/stat-smooth.R +++ b/R/stat-smooth.R @@ -171,14 +171,25 @@ StatSmooth <- ggproto("StatSmooth", Stat, method.args$method <- "REML" } - model <- inject(method( - formula, - data = data, - weights = weight, - !!!method.args - )) - - prediction <- predictdf(model, xseq, se, level) + prediction <- tryCatch( + { + model <- inject(method( + formula, + data = data, + weights = weight, + !!!method.args + )) + predictdf(model, xseq, se, level) + }, + error = function(cnd) { + cli::cli_warn("Failed to fit group {data$group[1]}.", parent = cnd) + NULL + } + ) + if (is.null(prediction)) { + return(NULL) + } + prediction$flipped_aes <- flipped_aes flip_data(prediction, flipped_aes) }, From 4bcd15100b1d346766045a243df1b8d2ee811862 Mon Sep 17 00:00:00 2001 From: Teun van den Brand <49372158+teunbrand@users.noreply.github.com> Date: Thu, 27 Jul 2023 19:52:58 +0200 Subject: [PATCH 2/5] Add test --- tests/testthat/test-geom-smooth.R | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/testthat/test-geom-smooth.R b/tests/testthat/test-geom-smooth.R index b0f4bbf44c..ca57bd2e38 100644 --- a/tests/testthat/test-geom-smooth.R +++ b/tests/testthat/test-geom-smooth.R @@ -77,6 +77,22 @@ test_that("default smoothing methods for small and large data sets work", { expect_equal(plot_data$y, as.numeric(out)) }) +test_that("geom_smooth() works when one group fails", { + # Group A fails, B succeeds + df <- data_frame0( + x = c(1, 2, 1, 2, 3), + y = c(1, 2, 3, 2, 1), + g = rep(c("A", "B"), 2:3) + ) + p <- ggplot(df, aes(x, y, group = g)) + + geom_smooth(method = "loess", formula = y ~ x) + + suppressWarnings( + expect_warning(ld <- layer_data(p), "Failed to fit group 1") + ) + expect_equal(unique(ld$group), 2) + expect_gte(nrow(ld), 2) +}) # Visual tests ------------------------------------------------------------ From 39c1ba3856b11226b44f3b88ec8dace0f267ded9 Mon Sep 17 00:00:00 2001 From: Teun van den Brand <49372158+teunbrand@users.noreply.github.com> Date: Thu, 27 Jul 2023 19:57:51 +0200 Subject: [PATCH 3/5] Add news bullet --- NEWS.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/NEWS.md b/NEWS.md index 6fb136dcfc..fe03835206 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,8 @@ # ggplot2 (development version) +* Failing to fit or predict in `stat_smooth()` now gives a warning and omits + the failed group, instead of throwing an error (@teunbrand, #5352). + * Nicer error messages for xlim/ylim arguments in coord-* functions (@92amartins, #4601, #5297). From 228464a5cdbc80dcd5135ce90c1829f857a77761 Mon Sep 17 00:00:00 2001 From: Teun van den Brand <49372158+teunbrand@users.noreply.github.com> Date: Thu, 27 Jul 2023 22:28:57 +0200 Subject: [PATCH 4/5] Guarantee clean error in test --- tests/testthat/test-stats.R | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/tests/testthat/test-stats.R b/tests/testthat/test-stats.R index 2cd71ab089..6c46bb38df 100644 --- a/tests/testthat/test-stats.R +++ b/tests/testthat/test-stats.R @@ -5,13 +5,9 @@ test_that("plot succeeds even if some computation fails", { b1 <- ggplot_build(p1) expect_equal(length(b1$data), 1) - p2 <- p1 + geom_smooth() + p2 <- p1 + stat_summary(fun = function(x) stop("Failed computation")) - # TODO: These multiple warnings should be summarized nicely. Until this gets - # fixed, this test ignores all the following errors than the first one. - suppressWarnings( - expect_warning(b2 <- ggplot_build(p2), "Computation failed") - ) + expect_warning(b2 <- ggplot_build(p2), "Computation failed") expect_equal(length(b2$data), 2) }) From 4165eb94dc4d6d54775a959651da2d76c459dba5 Mon Sep 17 00:00:00 2001 From: Teun van den Brand <49372158+teunbrand@users.noreply.github.com> Date: Tue, 1 Aug 2023 19:10:24 +0200 Subject: [PATCH 5/5] Use `try_fetch()` --- NEWS.md | 1 + R/stat-smooth.R | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 0cdd5ace25..6406a02c29 100644 --- a/NEWS.md +++ b/NEWS.md @@ -2,6 +2,7 @@ * Failing to fit or predict in `stat_smooth()` now gives a warning and omits the failed group, instead of throwing an error (@teunbrand, #5352). + * Integers are once again valid input to theme arguments that expect numeric input (@teunbrand, #5369) diff --git a/R/stat-smooth.R b/R/stat-smooth.R index 0081b380c8..a2180f2dc8 100644 --- a/R/stat-smooth.R +++ b/R/stat-smooth.R @@ -171,7 +171,7 @@ StatSmooth <- ggproto("StatSmooth", Stat, method.args$method <- "REML" } - prediction <- tryCatch( + prediction <- try_fetch( { model <- inject(method( formula,