From 7ec2bb13036d526d3bdd229151d210c03c5eca9b Mon Sep 17 00:00:00 2001 From: Gauarv Chaudhary Date: Fri, 24 Oct 2025 19:52:10 +0530 Subject: [PATCH 1/3] test: add failing tests for check_model() theme argument bug (#851) - Tests document expected behavior of theme argument - Tests currently FAIL due to bugs in see package - Covers: standard themes, custom themes, string themes, attribute storage - Bug is in see/R/plot.check_model.R (hardcoded args issue) Relates to #851 --- tests/testthat/test-check_model_theme.R | 217 ++++++++++++++++++++++++ 1 file changed, 217 insertions(+) create mode 100644 tests/testthat/test-check_model_theme.R diff --git a/tests/testthat/test-check_model_theme.R b/tests/testthat/test-check_model_theme.R new file mode 100644 index 000000000..8dc7c74a1 --- /dev/null +++ b/tests/testthat/test-check_model_theme.R @@ -0,0 +1,217 @@ +# ============================================================================== +# ISSUE #851: theme argument of check_model() has no effect +# ============================================================================== +# +# These tests document the expected behavior of the theme argument in +# check_model(). Currently, these tests FAIL due to bugs in the see package's +# plot.check_model() function. +# +# Three main problems identified: +# 1. Standard ggplot2 themes cause "unused arguments" errors +# 2. Custom user-defined theme functions cannot be accessed +# 3. Theme attribute not properly read by see package's plot method +# +# The actual fix needs to be implemented in the see package's +# R/plot.check_model.R file (around lines 64-67). +# +# See: https://github.com/easystats/performance/issues/851 +# ============================================================================== + +# Test 1: Standard ggplot2 themes should work without errors +# ----------------------------------------------------------------------------- +# PROBLEM: Currently throws "unused arguments" error because see package passes +# hardcoded arguments (plot.title.space, axis.title.space, etc.) that don't +# exist in standard ggplot2 themes. +test_that("check_model accepts standard ggplot2 themes as functions", { + skip_if_not_installed("performance") + skip_if_not_installed("see") + skip_if_not_installed("ggplot2") + + m <- lm(mpg ~ wt + cyl + gear + disp, data = mtcars) + + # Test with theme_dark passed as function (not string) + expect_no_error({ + p1 <- check_model(m, theme = ggplot2::theme_dark) + # The plot method should handle the theme attribute correctly + if (requireNamespace("see", quietly = TRUE)) { + plot(p1) + } + }) + + # Test with theme_minimal + expect_no_error({ + p2 <- check_model(m, theme = ggplot2::theme_minimal) + if (requireNamespace("see", quietly = TRUE)) { + plot(p2) + } + }) + + # Test with theme_bw + expect_no_error({ + p3 <- check_model(m, theme = ggplot2::theme_bw) + if (requireNamespace("see", quietly = TRUE)) { + plot(p3) + } + }) +}) + + +# Test 2: Theme as string should work for backward compatibility +# ----------------------------------------------------------------------------- +# PROBLEM: String parsing doesn't work reliably, and when it does parse, +# it still hits the "unused arguments" error from hardcoded parameters. +test_that("check_model accepts theme as string (backward compatibility)", { + skip_if_not_installed("performance") + skip_if_not_installed("see") + skip_if_not_installed("ggplot2") + + m <- lm(mpg ~ wt + cyl + gear + disp, data = mtcars) + + # String reference should work + expect_no_error({ + p1 <- check_model(m, theme = "ggplot2::theme_dark") + if (requireNamespace("see", quietly = TRUE)) { + plot(p1) + } + }) + + # Test with theme_bw as string + expect_no_error({ + p2 <- check_model(m, theme = "ggplot2::theme_bw") + if (requireNamespace("see", quietly = TRUE)) { + plot(p2) + } + }) +}) + + +# Test 3: Custom theme functions should work +# ----------------------------------------------------------------------------- +# PROBLEM: Custom themes defined in user environment can't be accessed because +# the see package's string parsing approach only searches package namespaces. +test_that("check_model accepts custom theme functions from user environment", { + skip_if_not_installed("performance") + skip_if_not_installed("see") + skip_if_not_installed("ggplot2") + + # Define custom theme in test environment + my_custom_theme <- function(base_size = 11, + base_family = "", + base_line_size = base_size / 22, + base_rect_size = base_size / 22) { + ggplot2::theme_minimal( + base_size = base_size, + base_family = base_family, + base_line_size = base_line_size, + base_rect_size = base_rect_size + ) + + ggplot2::theme( + plot.title = ggplot2::element_text(size = 16, face = "bold"), + panel.grid.major = ggplot2::element_line(color = "gray80") + ) + } + + m <- lm(mpg ~ wt + cyl + gear + disp, data = mtcars) + + # Custom theme function should work when passed directly + expect_no_error({ + p1 <- check_model(m, theme = my_custom_theme) + if (requireNamespace("see", quietly = TRUE)) { + plot(p1) + } + }) + + # Also test passing custom theme to plot method's style argument + expect_no_error({ + p2 <- check_model(m) + if (requireNamespace("see", quietly = TRUE)) { + plot(p2, style = my_custom_theme) + } + }) +}) + + +# Test 4: Theme attribute should be stored and retrieved correctly +# ----------------------------------------------------------------------------- +# PROBLEM: The theme attribute is stored by check_model() but may not be +# properly read by see package's plot method. +test_that("theme attribute is stored and used correctly", { + skip_if_not_installed("performance") + skip_if_not_installed("see") + skip_if_not_installed("ggplot2") + + m <- lm(mpg ~ wt + cyl + gear + disp, data = mtcars) + + # Test with function + p1 <- check_model(m, theme = ggplot2::theme_dark) + + # Theme should be stored as attribute + expect_true(!is.null(attr(p1, "theme"))) + + # Attribute should contain the theme + theme_attr <- attr(p1, "theme") + expect_true(is.function(theme_attr) || is.character(theme_attr)) + + # Theme should be applied when plotting + if (requireNamespace("see", quietly = TRUE)) { + expect_no_error(plot(p1)) + } + + # Test with string + p2 <- check_model(m, theme = "ggplot2::theme_minimal") + expect_true(!is.null(attr(p2, "theme"))) + expect_equal(attr(p2, "theme"), "ggplot2::theme_minimal") + + if (requireNamespace("see", quietly = TRUE)) { + expect_no_error(plot(p2)) + } +}) + + +# Test 5: Default theme should work when no theme specified +# ----------------------------------------------------------------------------- +test_that("check_model works without theme argument (default behavior)", { + skip_if_not_installed("performance") + skip_if_not_installed("see") + + m <- lm(mpg ~ wt + cyl + gear + disp, data = mtcars) + + # Should work with default theme + expect_no_error({ + p <- check_model(m) + if (requireNamespace("see", quietly = TRUE)) { + plot(p) + } + }) + + # Default theme should be stored + p <- check_model(m) + expect_true(!is.null(attr(p, "theme"))) +}) + + +# Test 6: style argument in plot() should override theme from check_model() +# ----------------------------------------------------------------------------- +# This tests that the plot method's style parameter can override the theme +# set during check_model() call. +test_that("plot style argument overrides check_model theme", { + skip_if_not_installed("performance") + skip_if_not_installed("see") + skip_if_not_installed("ggplot2") + + m <- lm(mpg ~ wt + cyl + gear + disp, data = mtcars) + + # Create with one theme + p <- check_model(m, theme = ggplot2::theme_dark) + + # Plot with different theme - should work + if (requireNamespace("see", quietly = TRUE)) { + expect_no_error({ + plot(p, style = ggplot2::theme_minimal) + }) + + expect_no_error({ + plot(p, style = ggplot2::theme_bw) + }) + } +}) From fca1404e365d1fb7eec03ecb89d87dc79a9922ea Mon Sep 17 00:00:00 2001 From: Gauarv Chaudhary Date: Fri, 24 Oct 2025 20:05:58 +0530 Subject: [PATCH 2/3] test: document theme argument bug and address review feedback (#851) Tests document expected behavior of theme argument: - Standard ggplot2 themes (theme_dark, theme_minimal, theme_bw) - Custom user-defined theme functions - Theme as string for backward compatibility - Theme attribute storage and retrieval Changes from review: - Remove trailing whitespace - Add namespace clarity comments - Use expect_identical for stricter comparison - Validate theme functions return theme objects - Add explanatory comment for default theme test Note: The actual bug is in the see package's plot.check_model() function which passes hardcoded arguments that don't exist in standard ggplot2 themes. This requires a fix in the see repository. Relates to #851 --- NEWS.md | 8 +++++ tests/testthat/test-check_model_theme.R | 48 ++++++++++++++----------- 2 files changed, 36 insertions(+), 20 deletions(-) diff --git a/NEWS.md b/NEWS.md index e67514eff..3ee855ae6 100644 --- a/NEWS.md +++ b/NEWS.md @@ -7,6 +7,14 @@ * Improved documentation for printing-methods. +## Known Issues + +* The `theme` argument in `check_model()` currently has no effect due to a bug + in the **see** package's plot method. The `check_model()` function correctly + stores the theme attribute, but the **see** package's `plot.check_model()` + function does not properly handle it. This issue is tracked in #851 and + requires a fix in the **see** package repository. + # performance 0.15.2 ## Bug fixes diff --git a/tests/testthat/test-check_model_theme.R b/tests/testthat/test-check_model_theme.R index 8dc7c74a1..36c8bdf82 100644 --- a/tests/testthat/test-check_model_theme.R +++ b/tests/testthat/test-check_model_theme.R @@ -1,7 +1,7 @@ # ============================================================================== # ISSUE #851: theme argument of check_model() has no effect # ============================================================================== -# +# # These tests document the expected behavior of the theme argument in # check_model(). Currently, these tests FAIL due to bugs in the see package's # plot.check_model() function. @@ -11,7 +11,7 @@ # 2. Custom user-defined theme functions cannot be accessed # 3. Theme attribute not properly read by see package's plot method # -# The actual fix needs to be implemented in the see package's +# The actual fix needs to be implemented in the see package's # R/plot.check_model.R file (around lines 64-67). # # See: https://github.com/easystats/performance/issues/851 @@ -26,10 +26,11 @@ test_that("check_model accepts standard ggplot2 themes as functions", { skip_if_not_installed("performance") skip_if_not_installed("see") skip_if_not_installed("ggplot2") - + m <- lm(mpg ~ wt + cyl + gear + disp, data = mtcars) - + # Test with theme_dark passed as function (not string) + # Explicit namespace call for clarity and to avoid conflicts expect_no_error({ p1 <- check_model(m, theme = ggplot2::theme_dark) # The plot method should handle the theme attribute correctly @@ -37,7 +38,7 @@ test_that("check_model accepts standard ggplot2 themes as functions", { plot(p1) } }) - + # Test with theme_minimal expect_no_error({ p2 <- check_model(m, theme = ggplot2::theme_minimal) @@ -64,9 +65,9 @@ test_that("check_model accepts theme as string (backward compatibility)", { skip_if_not_installed("performance") skip_if_not_installed("see") skip_if_not_installed("ggplot2") - + m <- lm(mpg ~ wt + cyl + gear + disp, data = mtcars) - + # String reference should work expect_no_error({ p1 <- check_model(m, theme = "ggplot2::theme_dark") @@ -93,7 +94,7 @@ test_that("check_model accepts custom theme functions from user environment", { skip_if_not_installed("performance") skip_if_not_installed("see") skip_if_not_installed("ggplot2") - + # Define custom theme in test environment my_custom_theme <- function(base_size = 11, base_family = "", @@ -139,18 +140,23 @@ test_that("theme attribute is stored and used correctly", { skip_if_not_installed("performance") skip_if_not_installed("see") skip_if_not_installed("ggplot2") - + m <- lm(mpg ~ wt + cyl + gear + disp, data = mtcars) - + # Test with function p1 <- check_model(m, theme = ggplot2::theme_dark) - + # Theme should be stored as attribute expect_true(!is.null(attr(p1, "theme"))) - + # Attribute should contain the theme theme_attr <- attr(p1, "theme") expect_true(is.function(theme_attr) || is.character(theme_attr)) + + # Validate theme more specifically + if (is.function(theme_attr)) { + expect_s3_class(theme_attr(), "theme") + } # Theme should be applied when plotting if (requireNamespace("see", quietly = TRUE)) { @@ -160,7 +166,7 @@ test_that("theme attribute is stored and used correctly", { # Test with string p2 <- check_model(m, theme = "ggplot2::theme_minimal") expect_true(!is.null(attr(p2, "theme"))) - expect_equal(attr(p2, "theme"), "ggplot2::theme_minimal") + expect_identical(attr(p2, "theme"), "ggplot2::theme_minimal") if (requireNamespace("see", quietly = TRUE)) { expect_no_error(plot(p2)) @@ -170,12 +176,14 @@ test_that("theme attribute is stored and used correctly", { # Test 5: Default theme should work when no theme specified # ----------------------------------------------------------------------------- +# Tests that check_model works correctly when no theme argument is provided, +# ensuring it falls back to the default theme. test_that("check_model works without theme argument (default behavior)", { skip_if_not_installed("performance") skip_if_not_installed("see") - + m <- lm(mpg ~ wt + cyl + gear + disp, data = mtcars) - + # Should work with default theme expect_no_error({ p <- check_model(m) @@ -183,7 +191,7 @@ test_that("check_model works without theme argument (default behavior)", { plot(p) } }) - + # Default theme should be stored p <- check_model(m) expect_true(!is.null(attr(p, "theme"))) @@ -198,18 +206,18 @@ test_that("plot style argument overrides check_model theme", { skip_if_not_installed("performance") skip_if_not_installed("see") skip_if_not_installed("ggplot2") - + m <- lm(mpg ~ wt + cyl + gear + disp, data = mtcars) - + # Create with one theme p <- check_model(m, theme = ggplot2::theme_dark) - + # Plot with different theme - should work if (requireNamespace("see", quietly = TRUE)) { expect_no_error({ plot(p, style = ggplot2::theme_minimal) }) - + expect_no_error({ plot(p, style = ggplot2::theme_bw) }) From bf21c1e861480c8d6b2b75282846a7c6c3e2746f Mon Sep 17 00:00:00 2001 From: Gauarv Chaudhary Date: Fri, 24 Oct 2025 20:20:21 +0530 Subject: [PATCH 3/3] removed the whitespace --- tests/testthat/test-check_model_theme.R | 26 +++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/tests/testthat/test-check_model_theme.R b/tests/testthat/test-check_model_theme.R index 36c8bdf82..08904bb1a 100644 --- a/tests/testthat/test-check_model_theme.R +++ b/tests/testthat/test-check_model_theme.R @@ -46,7 +46,7 @@ test_that("check_model accepts standard ggplot2 themes as functions", { plot(p2) } }) - + # Test with theme_bw expect_no_error({ p3 <- check_model(m, theme = ggplot2::theme_bw) @@ -75,7 +75,7 @@ test_that("check_model accepts theme as string (backward compatibility)", { plot(p1) } }) - + # Test with theme_bw as string expect_no_error({ p2 <- check_model(m, theme = "ggplot2::theme_bw") @@ -96,10 +96,12 @@ test_that("check_model accepts custom theme functions from user environment", { skip_if_not_installed("ggplot2") # Define custom theme in test environment - my_custom_theme <- function(base_size = 11, - base_family = "", - base_line_size = base_size / 22, - base_rect_size = base_size / 22) { + my_custom_theme <- function( + base_size = 11, + base_family = "", + base_line_size = base_size / 22, + base_rect_size = base_size / 22 + ) { ggplot2::theme_minimal( base_size = base_size, base_family = base_family, @@ -111,9 +113,9 @@ test_that("check_model accepts custom theme functions from user environment", { panel.grid.major = ggplot2::element_line(color = "gray80") ) } - + m <- lm(mpg ~ wt + cyl + gear + disp, data = mtcars) - + # Custom theme function should work when passed directly expect_no_error({ p1 <- check_model(m, theme = my_custom_theme) @@ -121,7 +123,7 @@ test_that("check_model accepts custom theme functions from user environment", { plot(p1) } }) - + # Also test passing custom theme to plot method's style argument expect_no_error({ p2 <- check_model(m) @@ -157,17 +159,17 @@ test_that("theme attribute is stored and used correctly", { if (is.function(theme_attr)) { expect_s3_class(theme_attr(), "theme") } - + # Theme should be applied when plotting if (requireNamespace("see", quietly = TRUE)) { expect_no_error(plot(p1)) } - + # Test with string p2 <- check_model(m, theme = "ggplot2::theme_minimal") expect_true(!is.null(attr(p2, "theme"))) expect_identical(attr(p2, "theme"), "ggplot2::theme_minimal") - + if (requireNamespace("see", quietly = TRUE)) { expect_no_error(plot(p2)) }