Skip to content

Fix duplicate aesthetics warning in ggsurv() for upcoming ggplot2#572

Merged
schloerke merged 4 commits intotweaksfrom
copilot/add-news-entry-for-warning
Feb 4, 2026
Merged

Fix duplicate aesthetics warning in ggsurv() for upcoming ggplot2#572
schloerke merged 4 commits intotweaksfrom
copilot/add-news-entry-for-warning

Conversation

Copy link
Copy Markdown

Copilot AI commented Feb 2, 2026

ggplot2's upcoming release warns when aesthetics are specified both in aes() and as fixed parameters. ggsurv() specified lty in both locations for confidence interval bounds, triggering warnings that fail the expect_silent() test.

Changes

  • R/ggsurv.R: Removed lty = .data$group from aes() in confidence interval geom_step() calls (lines 335, 340). The fixed lty = stepLty parameter remains and provides the intended behavior.
  • NEWS.md: Added entry documenting the fix

Before/After

# Before - duplicate lty specification
geom_step(
  aes(y = .data$up, lty = .data$group, col = .data$group),  # lty here
  lty = stepLty,                                             # and here
  linewidth = size.ci
)

# After - single lty specification
geom_step(
  aes(y = .data$up, col = .data$group),
  lty = stepLty,
  linewidth = size.ci
)

Verified that removing the aesthetic mapping does not change plot output, as noted by @schloerke.

Original prompt

This section details on the original issue you should resolve

<issue_title>Heads up: duplicate aesthetics will get warning</issue_title>
<issue_description>Hi Barett and other maintainers,

We recently did a revdepcheck in ggplot2 and found that a new check we introduced got triggered by GGally. In particular we now throw a warning which violates this test:

expect_silent(
ggsurv(sf.kid, CI = TRUE, surv.col = c("black", "red", "blue", "green"))
)

I believe the warning is thrown because lty is used twice in these layer definitions, once in aes() and once as a fixed parameter:

ggally/R/ggsurv.R

Lines 333 to 343 in d21c6d6

pl <- pl +
geom_step(
aes(y = .data$up, lty = .data$group, col = .data$group),
lty = stepLty,
linewidth = size.ci
) +
geom_step(
aes(y = .data$low, lty = .data$group, col = .data$group),
lty = stepLty,
linewidth = size.ci
)

We've no immediate plans to release a ggplot2 that includes the check, but just wanted to simply flag this for the future.

Cheers!</issue_description>

<agent_instructions>Please add a NEWS.md entry</agent_instructions>

Comments on the Issue (you are @copilot in this section)

@schloerke Thank you!

Local testing shows that if we remove aes(lty= .data$group), the plots are the same. We should keep the geom_step arg.

Before making any changes, we should verify that an error is thrown in the test when using the dev version of ggplot2


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 2 commits February 2, 2026 16:51
Co-authored-by: schloerke <93231+schloerke@users.noreply.github.com>
Co-authored-by: schloerke <93231+schloerke@users.noreply.github.com>
Copilot AI changed the title [WIP] Add NEWS.md entry for duplicate aesthetics warning Fix duplicate aesthetics warning in ggsurv() for upcoming ggplot2 Feb 2, 2026
Copilot AI requested a review from schloerke February 2, 2026 16:53
@schloerke schloerke changed the base branch from main to tweaks February 4, 2026 02:34
@schloerke schloerke marked this pull request as ready for review February 4, 2026 02:48
@schloerke schloerke merged commit d8623b6 into tweaks Feb 4, 2026
1 of 4 checks passed
@schloerke schloerke deleted the copilot/add-news-entry-for-warning branch February 4, 2026 02:48
@schloerke schloerke mentioned this pull request Feb 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Heads up: duplicate aesthetics will get warning

2 participants