Fix aes_string() deprecation warning in diagnostics plots (#582) - #584
Draft
kalra-mohit wants to merge 1 commit into
Draft
Fix aes_string() deprecation warning in diagnostics plots (#582)#584kalra-mohit wants to merge 1 commit into
kalra-mohit wants to merge 1 commit into
Conversation
…ted#582) ggplot2 >= 3.0.0 hard-deprecates aes_string(), which plot.model_diagnostics() and plot.predict_diagnostics() both called directly, causing every diagnostics plot to emit a lifecycle deprecation warning. Replaced with tidy evaluation (aes() + .data[[...]]), the idiom ggplot2's own deprecation message recommends. plot.predict_diagnostics() had the same latent issue in its histogram branch (variables = NULL), even though it wasn't called out in the issue title. Added regression tests to test_model_diagnostics.R and test_predict_diagnostics.R that assert the plotting functions' source no longer references aes_string() (checked structurally rather than by capturing the warning at call time, since lifecycle warnings are only emitted once per R session and a warning-capturing test would silently pass on subsequent calls regardless of whether the bug is fixed). Verified: both new tests fail against the pre-fix source and pass after the fix; existing test_model_diagnostics.R / test_predict_diagnostics.R suites still pass in full with no aes_string warnings. Closes ModelOriented#582 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
mayer79
reviewed
Jul 23, 2026
| nlabels <- length(unique(all_models$label)) | ||
|
|
||
| pl <- ggplot(all_models, aes_string(x = variable, y = yvariable, color = "label", group = "label")) + | ||
| pl <- ggplot(all_models, aes(x = .data[[variable]], y = .data[[yvariable]], color = .data[["label"]], group = .data[["label"]])) + |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ggplot2>= 3.0.0 hard-deprecatesaes_string(). Bothplot.model_diagnostics()andplot.predict_diagnostics()called it directly, so every diagnostics plot emitted alifecycledeprecation warning:Reproduced exactly using the reprex from #582:
Root cause / fix: both functions built their
aes()mapping with variable-name strings (column names passed in as function arguments, e.g.variable = "y_hat"), which is exactly whataes_string()was for before tidy evaluation existed. The fix swaps inaes()+.data[[...]], which is the direct replacementggplot2's own deprecation message points to, and keeps the "map by column-name string" behavior these functions depend on (variable/yvariableare user-facing string parameters).While tracing this, I found
plot.predict_diagnostics()'s histogram branch (used whenvariables = NULL) has the exact same latent bug — not mentioned in the issue title, but same root cause and same file area, so fixed it too rather than leaving a near-identical bug in place.flowchart LR A["model_diagnostics(explainer)"] --> B["plot.model_diagnostics()"] B --> C["ggplot(all_models, aes_string(...))\n⚠ deprecated in ggplot2 >= 3.0.0"] C -->|"this PR"| D["ggplot(all_models, aes(x = .data[[variable]], ...))"] E["predict_diagnostics(explainer, ..., variables = NULL)"] --> F["plot.predict_diagnostics()\n(histogram branch)"] F --> G["ggplot(df, aes_string(...))\n⚠ same deprecation"] G -->|"this PR"| H["ggplot(df, aes(x = .data[[\"Var1\"]], ...))"]Why it matters
model_diagnostics()/plot()andpredict_diagnostics()/plot()are core, commonly-used entry points (residual diagnostics, instance-level diagnostics). The warning fires on every single call, which is noisy for users and — sincelifecyclewarnings can be promoted to errors byoptions(lifecycle_verbosity = "error")or under strict CRAN checks on newerggplot2— a forward-compatibility risk, not just cosmetic noise.Test plan
plot.model_diagnostics()gives a warning for using deprecatedaes_string()with ggplot2 >=3.0.0 #582's reprex against currentmaster.tests/testthat/test_model_diagnostics.Randtests/testthat/test_predict_diagnostics.Rthat asserts the plotting functions' source no longer referencesaes_string().expect_warning/expect_no_warning, becauselifecycledeprecation warnings are only emitted once per R session — whichever test callsplot.model_diagnostics()first "uses up" the warning, so a warning-capturing test would silently pass on later calls in the same test run regardless of whether the underlying bug is fixed. Confirmed this empirically before choosing the structural check.failed: 1) and pass after the fix (failed: 0), by checking out the pre-fix version of the twoR/files, rebuilding, and rerunningtestthat::test_file()on just the two affected test files, then restoring the fix and rerunning.test_model_diagnostics.Randtest_predict_diagnostics.Rsuites (not just the new tests) after the fix — all pass, and the pre-existing"Plot"tests no longer emit theaes_string()warning either.ggobject, correct columns bound to x/y/color/fill) — this is a call-site swap, not a behavior change.Ran only the two affected test files locally (per this session's scoped-testing convention); full
R CMD check/ CI will run the complete suite.Closes #582