-
-
Notifications
You must be signed in to change notification settings - Fork 46
Add vdiffr snapshot tests for diagnostic plots (#236) #423
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Add vdiffr snapshot tests for diagnostic plots (#236) #423
Conversation
Added 18 snapshot tests using vdiffr to ensure diagnostic plots render consistently across package versions. This helps catch visual regressions before they reach users. Tests cover check_model(), check_normality(), check_outliers(), and check_homogeneity() with various themes, colors, and options. Also validates that the large dataset performance fix produces visually consistent output. Fixes easystats#236
Summary of ChangesHello @ANAMASGARD, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the robustness of the plotting functions by adding a suite of Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a comprehensive suite of snapshot tests for diagnostic plots using the vdiffr package. The tests cover various functions like check_model(), check_normality(), check_homogeneity(), and check_outliers() with different options, themes, and even for large datasets, which is excellent for ensuring visual consistency and preventing regressions. The test code is well-structured and clearly commented. I have one suggestion to improve code maintainability by reducing repetitive set.seed() calls, which will make the test suite even cleaner.
| test_that("plot.see_check_model() renders correctly", { | ||
| skip_if_not_installed("performance") | ||
| skip_if_not_installed("see") | ||
| skip_on_cran() | ||
|
|
||
| set.seed(123) | ||
| model <- lm(mpg ~ wt + hp + cyl, data = mtcars) | ||
|
|
||
| # Test linearity check (NCV) | ||
| set.seed(123) | ||
| vdiffr::expect_doppelganger( | ||
| title = "check_model_linearity", | ||
| fig = plot(performance::check_model(model, check = "linearity")) | ||
| ) | ||
|
|
||
| # Test homogeneity of variance | ||
| set.seed(123) | ||
| vdiffr::expect_doppelganger( | ||
| title = "check_model_homogeneity", | ||
| fig = plot(performance::check_model(model, check = "homogeneity")) | ||
| ) | ||
|
|
||
| # Test normality (QQ plot) | ||
| set.seed(123) | ||
| vdiffr::expect_doppelganger( | ||
| title = "check_model_qq", | ||
| fig = plot(performance::check_model(model, check = "qq")) | ||
| ) | ||
|
|
||
| # Test outliers | ||
| set.seed(123) | ||
| vdiffr::expect_doppelganger( | ||
| title = "check_model_outliers", | ||
| fig = plot(performance::check_model(model, check = "outliers")) | ||
| ) | ||
|
|
||
| # Test full panel plot | ||
| set.seed(123) | ||
| vdiffr::expect_doppelganger( | ||
| title = "check_model_panel", | ||
| fig = plot(performance::check_model(model)) | ||
| ) | ||
| }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test code is well-structured and covers various aspects of the plotting functions. However, there is a lot of repetition with set.seed(123) before each call to vdiffr::expect_doppelganger(). While ensuring reproducibility is crucial, this pattern makes the code verbose and harder to maintain.
To improve readability and reduce redundancy, you could introduce a small helper function that wraps set.seed() and vdiffr::expect_doppelganger(). This would make the test suite cleaner and more concise.
Here's an example of what the helper function could look like:
# Helper for reproducible vdiffr tests
expect_doppelganger_with_seed <- function(title, fig, seed = 123) {
set.seed(seed)
vdiffr::expect_doppelganger(title = title, fig = fig)
}You could then refactor this test_that() block as follows. This pattern can be applied to the other test blocks in this file as well.
test_that("plot.see_check_model() renders correctly", {
skip_if_not_installed("performance")
skip_if_not_installed("see")
skip_on_cran()
# Helper for reproducible vdiffr tests
expect_doppelganger_with_seed <- function(title, fig, seed = 123) {
set.seed(seed)
vdiffr::expect_doppelganger(title = title, fig = fig)
}
set.seed(123)
model <- lm(mpg ~ wt + hp + cyl, data = mtcars)
# Test linearity check (NCV)
expect_doppelganger_with_seed(
title = "check_model_linearity",
fig = plot(performance::check_model(model, check = "linearity"))
)
# Test homogeneity of variance
expect_doppelganger_with_seed(
title = "check_model_homogeneity",
fig = plot(performance::check_model(model, check = "homogeneity"))
)
# Test normality (QQ plot)
expect_doppelganger_with_seed(
title = "check_model_qq",
fig = plot(performance::check_model(model, check = "qq"))
)
# Test outliers
expect_doppelganger_with_seed(
title = "check_model_outliers",
fig = plot(performance::check_model(model, check = "outliers"))
)
# Test full panel plot
expect_doppelganger_with_seed(
title = "check_model_panel",
fig = plot(performance::check_model(model))
)
})Implemented Gemini AI code review suggestion to reduce code repetition by introducing expect_doppelganger_with_seed() helper function.
Code Quality ImprovementsImplemented Gemini AI code review suggestions:
|
|
Thanks a lot, let me review this PR! |
I've added 18 vdiffr tests that cover the main check_model() plots and their variations. These should catch any visual regressions in future updates.
Tests cover check_model(), check_normality(), check_outliers(), and check_homogeneity() with various themes, colors, and options. Also validates that the large dataset performance fix produces visually consistent output.
I followed the tutorial from https://indrajeetpatil.github.io/intro-to-snapshot-testing/ to learn how vdiffr works.
Contributes to #236