Skip to content

GH-49533: [R] Implement dplyr's when_any() and when_all() helpers#49535

Merged
thisisnic merged 3 commits into
apache:mainfrom
thisisnic:GH-49533_dplyr_when_helpers
Mar 27, 2026
Merged

GH-49533: [R] Implement dplyr's when_any() and when_all() helpers#49535
thisisnic merged 3 commits into
apache:mainfrom
thisisnic:GH-49533_dplyr_when_helpers

Conversation

@thisisnic

@thisisnic thisisnic commented Mar 17, 2026

Copy link
Copy Markdown
Member

Rationale for this change

Bindings to new dplyr helpers

What changes are included in this PR?

Bindings and tests

Are these changes tested?

Yes

Are there any user-facing changes?

Yes

AI usage

I have written this using Claude Opus 4.5 using Claude from the cli. I have manually reviewed the changes myself and am happy with them.

@thisisnic thisisnic marked this pull request as ready for review March 24, 2026 13:17
@thisisnic thisisnic requested a review from jonkeane as a code owner March 24, 2026 13:17
@thisisnic

Copy link
Copy Markdown
Member Author

@jonkeane - mind giving this a look over if you have the time?

@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 for this. One small question, but this looks good

Comment on lines +530 to +536
# na_rm=TRUE treats NA as FALSE
compare_dplyr_binding(
.input |>
mutate(result = when_any(lgl, false, na_rm = TRUE)) |>
collect(),
tbl
)

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 assume that lgl here has NAs and that's what is being tested here, right?

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.

Yep!

@github-actions github-actions Bot added awaiting merge Awaiting merge and removed awaiting committer review Awaiting committer review labels Mar 26, 2026
@thisisnic thisisnic merged commit 4bcab12 into apache:main Mar 27, 2026
17 of 18 checks passed
@thisisnic thisisnic removed the awaiting merge Awaiting merge label Mar 27, 2026
@conbench-apache-arrow

Copy link
Copy Markdown

After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit 4bcab12.

There was 1 benchmark result with an error:

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

thisisnic added a commit to thisisnic/arrow that referenced this pull request Apr 6, 2026
…rs (apache#49535)

### Rationale for this change

Bindings to new dplyr helpers

### What changes are included in this PR?

Bindings and tests

### Are these changes tested?

Yes

### Are there any user-facing changes?

Yes

### AI usage

I have written this using Claude Opus 4.5 using Claude from the cli.  I have manually reviewed the changes myself and am happy with them.

* GitHub Issue: apache#49533

Authored-by: Nic Crane <thisisnic@gmail.com>
Signed-off-by: Nic Crane <thisisnic@gmail.com>
Mottl pushed a commit to Mottl/arrow that referenced this pull request May 26, 2026
…rs (apache#49535)

### Rationale for this change

Bindings to new dplyr helpers

### What changes are included in this PR?

Bindings and tests

### Are these changes tested?

Yes

### Are there any user-facing changes?

Yes

### AI usage

I have written this using Claude Opus 4.5 using Claude from the cli.  I have manually reviewed the changes myself and am happy with them.

* GitHub Issue: apache#49533

Authored-by: Nic Crane <thisisnic@gmail.com>
Signed-off-by: Nic Crane <thisisnic@gmail.com>
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.

2 participants