Skip to content

Take a grouped slice's rows in one native call - #458

Merged
jbearak merged 2 commits into
mainfrom
claude/slice-rank-plan
Oct 27, 2026
Merged

jbearak merged 2 commits into
mainfrom
claude/slice-rank-plan

Conversation

@jbearak

@jbearak jbearak commented Oct 27, 2026

Copy link
Copy Markdown
Owner

Summary

slice_min(), slice_max(), slice_head() and slice_tail() of a grouped dibble evaluated each group in the data mask. slice_min() and slice_max() read the group's chunk of order_by, ranked it with vctrs::vec_rank(), and kept the rows ranked within the slice's size. slice_head() and slice_tail() took the group's first or last rows. Over the 42,739 women in the first 100,000 rows of the India 2021 DHS births file, slice_max(b, b2, n = 1, by = caseid) took about 1.0 seconds, where a tibble takes about 1.2, and slice_head(b, n = 1, by = caseid) about 0.14, where a tibble takes about 0.63.

This PR takes the rows of every group after the first in one native call once the loop has evaluated the first group:

  • C_dtatools_slice_position_plan() copies each group's first or last rows for slice_head() and slice_tail().
  • C_dtatools_slice_rank_plan() handles slice_min(x) and slice_max(x), where x names a compact Stata numeric column that only the first group has read. It reads the key of each of the column's rows once, as ADR 0104's plan reads a chunk's identity proxy, and sorts each group's keys, flipped for slice_max(), keeping ties in order. With with_ties = TRUE a row takes the rank of the first row it ties with, as vec_rank()'s "min" ties give it, and otherwise its own, as "sequential" ties give it. Each group keeps its rows ranked within the slice's size, ordered by rank and then position.
  • The slice's size for a group depends only on n or prop and the group's size when n or prop is a bare number. The plan then calls the size function once for each size the groups have. A classed or ALTREP number, or a size whose computation warns or errors, leaves the groups to the loop.

slice_min() and slice_max() also need ADR 0104's checks that the loop's vec_rank() call would give these ranks: the records of the functions it reaches, the routines dtatools captured at load, and the absence of other order or proxy methods. vec_rank() with incomplete = "rank", as the slice calls it, finds no complete rows and calls no equality method, so .order_proxy_admitted() takes complete = FALSE to leave those methods unchecked. None of the closures the loop calls for a later group may be debugged.

Behavior

Results are unchanged. ADR 0120 records the decision. Rowwise data, slice_sample(), na_rm = TRUE, an order_by that is not a column's name, and rows that are not bare integer vectors leave the groups to the loop. A traced or replaced vec_check_size(), which(), order() or %in% sees only the first group's calls, as ADR 0058 allows for helpers whose results the plan reproduces.

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. Before is main.

Call Before After Tibble
slice_max(b, b2, n = 1, by = caseid) 1.0 0.010 1.2
slice_head(b, n = 1, by = caseid) 0.14 0.009 0.63

Tests

  • A test runs slice_min() and slice_max() of six Stata numeric columns, with and without ties, seven sizes of both signs, as n and prop, by group_by() and by by, 336 cases. The data's 60 groups interleave and hold ties, zeros of both signs, and missing and tagged missing values. Each case must give the same value, warnings and errors with the plan as without it, and the plan must take every one.
  • A test does the same for slice_head() and slice_tail(), 28 cases.
  • A test checks that the plan takes the empty groups of .drop = FALSE, and leaves these to the loop: na_rm = TRUE, expressions, pronouns, double and string columns, a name that is no column, rowwise and ungrouped data, slice_sample(), a classed proportion whose method reads the current group, an ALTREP n whose third read errors, and an integer proportion whose product with a size overflows, also under options(warn = 2). An order or proxy method in the global environment leaves the groups to the loop, but an equality or comparison method, which vec_rank() does not call here, does not.
  • A test traces vec_rank(), debugs each closure the plan skips, and replaces the routine vec_rank() calls, and checks that the plan declines.
  • The full suite passes: 169 files, 2,270 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 00:36
@jbearak
jbearak merged commit ae4bbff 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