Skip to content

GH-34860: [R] New column name wrongly set when using mutate with if_any - #51314

Merged
thisisnic merged 2 commits into
apache:mainfrom
thisisnic:GH-34860-mutate-if_any
Sep 17, 2026
Merged

thisisnic merged 2 commits into
apache:mainfrom
thisisnic:GH-34860-mutate-if_any

Conversation

@thisisnic

@thisisnic thisisnic commented Sep 12, 2026 •

Copy link
Copy Markdown
Member

Rationale for this change

We got errors with new column names when using mutate() with if_any() due to not having access to column name

What changes are included in this PR?

Set the names

Are these changes tested?

Yeah

Are there any user-facing changes?

Yeah

Copilot AI lite review requested due to automatic review settings September 12, 2026 13:23
@thisisnic
thisisnic requested a review from jonkeane as a code owner September 12, 2026 13:23
@github-actions

Copy link
Copy Markdown

⚠️ GitHub issue #34860 has been automatically assigned in GitHub to PR creator.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No unresolved review comments, and the fix includes focused regression coverage.

Pull request overview

Fixes R mutate() column naming for if_any() and if_all() by preserving user-provided names.

Changes:

  • Preserve names during predicate expansion.
  • Collapse predicates independently.
  • Add regression tests for naming and mixed expressions.
File summaries
File Summary
r/tests/testthat/test-dplyr-mutate.R Tests resulting mutate column names.
r/tests/testthat/test-dplyr-across.R Tests predicate expansion and name preservation.
r/R/dplyr-across.R Updates predicate expansion to retain names.
Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@thisisnic
thisisnic force-pushed the GH-34860-mutate-if_any branch from b1cbb4f to 928afe5 Compare September 16, 2026 14:34
Copilot AI review requested due to automatic review settings September 16, 2026 14:34

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The reviewed changes address the naming issue and include regression coverage.

Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@jonkeane jonkeane left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

Comment thread r/R/dplyr-across.R Outdated
# if_any()/if_all() collapse the expanded quosures into a single
# expression, keeping whatever name the user gave the call
if (is_call(quo_expr, c("if_any", "if_all"))) {
op <- if (is_call(quo_expr, "if_any")) "|" else "&"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine to run with it, but the inline if/else is a little 🤢

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're not wrong, I'll update

Copilot AI review requested due to automatic review settings September 17, 2026 17:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No unresolved issues were identified.

Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Copilot AI review requested due to automatic review settings September 17, 2026 17:30
@thisisnic
thisisnic force-pushed the GH-34860-mutate-if_any branch from 4c1802d to 59744ab Compare September 17, 2026 17:30

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No unresolved issues were identified, and all reviewed changes have regression coverage.

Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@github-actions github-actions Bot added awaiting merge Awaiting merge and removed awaiting committer review Awaiting committer review labels Sep 17, 2026
@thisisnic
thisisnic merged commit 46b6f9d into apache:main Sep 17, 2026
35 checks passed
@thisisnic thisisnic removed the awaiting merge Awaiting merge label Sep 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants