Skip to content

Take a grouped slice() of numbers in one native call - #460

Merged
jbearak merged 1 commit into
mainfrom
claude/slice-literal-plan
Oct 27, 2026
Merged

jbearak merged 1 commit into
mainfrom
claude/slice-literal-plan

Conversation

@jbearak

@jbearak jbearak commented Oct 27, 2026

Copy link
Copy Markdown
Owner

Summary

slice() of grouped data evaluates its expressions in the data mask for each group, casts each value with vctrs::vec_as_subscript() and combines a group's values with vctrs::vec_c(). It then turns each group's subscripts into the group's rows with vctrs::num_as_location(), which removes zeros, missing values and positions past the group's end. Each group's evaluation builds a new data mask. Over the 42,739 women in the first 100,000 rows of the India 2021 DHS births file, grouped by caseid, slice(gb, 1) took about 0.45 seconds, where a tibble takes about 0.36.

This PR adds C_dtatools_slice_literal_plan(). After the loop evaluates the first group, .slice_literal_planned() takes the rows of every later group in one call when each expression gives every group the value it gave the first. An expression does when it is one of these:

  • a vector of numbers or logical values without attributes, which no ALTREP class reads
  • a call of base's unary minus or ( on such an expression
  • a call of c() on any number of them, untagged
  • a call of : on two numbers of length 1 or their negations

That covers slice(gb, 1), slice(gb, -1), slice(gb, 1:3) and slice(gb, c(1, 3)). Each function the call finds from the quosure's environment must be base's primitive, and no column may be named for it, since R reads a column while it looks up a function of that name. None of these calls dispatches on values without a class or can warn. A binary minus's integer overflow and : of a longer vector do warn, so those leave the groups to the loop.

Each later group's subscripts are then the first group's. The call keeps the group's rows at the locations num_as_location() gives for the group's size: the positive positions within it in order, repeats kept, or every position the negative ones do not name, in order. Positive and negative positions together leave the groups to the loop, which raises the error in the first group.

Behavior

Results are unchanged. ADR 0122 records the decision. None of the closures the loop calls for a later group may be debugged: lapply(), environment(), vec_as_subscript(), vec_c(), num_as_location(), and the check_dots_empty0(), list2(), is_integer() and is_double() that vctrs' namespace finds. A traced or replaced one sees only the first group's calls, as ADR 0058 allows.

Timing

Best of three runs in fresh R sessions, in seconds, with the builds interleaved, over the first 100,000 rows of the India 2021 DHS births file grouped by caseid. Before is main.

Call Before After Tibble
slice(gb, 1) 0.45 0.016 0.36
slice(gb, -1) 0.46 0.016 0.48
slice(gb, 1:2) 0.46 0.017
slice(gb, c(1, 3)) 0.47 0.017

Tests

  • A test runs 25 subscripts, such as 1, -2L, 20, c(3, 1, 3), 5:2, -1:-3, -c(1, 3), c(1, NA), c() and an injected vector, on 60 interleaved groups and on the empty groups of .drop = FALSE. Each must give the same value, warnings and errors with the plan as without it, and the plan must take every one. It also runs more than one expression, none, .by and rowwise data.
  • A test runs 60 random slices of integer or double vectors of one sign with missing values over random groups, some empty.
  • A test runs cases the plan leaves to the loop: positions of both signs and a fraction, which error in the first group, a column, n(), a binary minus, names, a classed number, a column named c beside a call of c(), ungrouped data, : of a longer vector, and a minus in the global environment. A column named c does not stop slice(named, 1), which calls no c().
  • A test debugs each closure the plan skips while the plan decides, and checks that the plan declines.
  • The full suite passes: 169 files, 2,276 tests. The native manifest and repository guard tests pass. Codex review is clean.

@coderabbitai

coderabbitai Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@jbearak
jbearak marked this pull request as ready for review October 27, 2026 01:28
@jbearak
jbearak merged commit 9478eb3 into main Oct 27, 2026
4 checks passed
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.

1 participant