Skip to content

feat(pivotp): --agg quantile@<p> (alias q@<p>) with linear interpolation#3842

Merged
jqnatividad merged 6 commits into
masterfrom
pivotp-quantile
May 10, 2026
Merged

feat(pivotp): --agg quantile@<p> (alias q@<p>) with linear interpolation#3842
jqnatividad merged 6 commits into
masterfrom
pivotp-quantile

Conversation

@jqnatividad
Copy link
Copy Markdown
Collaborator

Summary

  • Adds per-group quantile aggregation to pivotp via a new --agg quantile@<p> aggregation function (alias q@<p>), where p is a float in [0, 1]. Examples: --agg quantile@0.95, --agg q@0.5.
  • Backed by Polars' Expr::quantile with QuantileMethod::Linear — exact, fast, and matches numpy/pandas/scipy/stats --quantile-method linear defaults. No new dependency.
  • Works in both pivot mode and group-by mode. Malformed probabilities (out of range, non-numeric) are caught by a precise pre-match validation error rather than the generic "Invalid pivot aggregation function" message.
  • Method override (quantile@p@method) is intentionally deferred — the surface stays minimal until someone asks for it.

(Supersedes #3841, which was auto-closed when the branch was renamed from pivotp-quantlle to fix a typo.)

Test plan

  • cargo build --locked --bin qsv -F all_features — clean
  • cargo t pivotp -F all_features — 62/62 pass
  • cargo clippy -F all_features — no warnings
  • Four new tests added in tests/test_pivotp.rs:
    • pivotp_quantile_p95_agg — pivot mode, --agg quantile@0.95, asserts exact linear-interpolated p95
    • pivotp_q_alias_p50_equals_median--agg q@0.5 produces byte-identical output to --agg median on the standard fixture
    • pivotp_quantile_invalid_p_rejectedquantile@1.5, q@-0.1, and quantile@abc all exit non-zero with "Invalid quantile probability" in stderr
    • pivotp_quantile_groupby_mode — group-by mode (no <on-cols>), --agg q@0.25, asserts Q1 per group

🤖 Generated with Claude Code

Adds per-group quantile aggregation in both pivot and group-by modes via
Polars' Expr::quantile with QuantileMethod::Linear (matches numpy/pandas/
scipy defaults). Probability p must be a float in [0, 1]; malformed values
hit a precise pre-match validation error rather than the generic "Invalid
pivot aggregation function". No new dependency.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@codacy-production
Copy link
Copy Markdown

codacy-production Bot commented May 10, 2026

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 10 complexity

Metric Results
Complexity 10

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

@jqnatividad jqnatividad requested a review from Copilot May 10, 2026 17:59
Comment thread tests/test_pivotp.rs Fixed
Comment thread tests/test_pivotp.rs Fixed
Comment thread tests/test_pivotp.rs Fixed
Comment thread tests/test_pivotp.rs Fixed
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a new per-group quantile aggregation option to the pivotp command (--agg quantile@<p> with alias q@<p>) implemented via Polars’ linear-interpolated quantile, including targeted validation errors and integration tests.

Changes:

  • Document quantile@<p> / q@<p> in pivotp help text.
  • Add quantile@<p> parsing/validation and wire it into both pivot-mode and group-by-mode aggregation expression building.
  • Add integration tests covering correct results, alias behavior, invalid probabilities, and group-by mode.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/cmd/pivotp.rs Adds parsing/validation for quantile@<p> / q@<p> and constructs Polars quantile expressions in pivot + group-by modes.
tests/test_pivotp.rs Adds new integration tests verifying quantile output, alias equivalence, invalid input rejection, and group-by behavior.

Comment thread src/cmd/pivotp.rs Outdated
Comment thread src/cmd/pivotp.rs Outdated
jqnatividad and others added 3 commits May 10, 2026 14:07
- Refactor both quantile match arms to `s if let Some(p) = parse_quantile_agg(s)`,
  parsing once and removing the .unwrap() + safety comment.
- Restore `mut` on the second cmd binding in pivotp_quantile_invalid_p_rejected
  (a hook had snake_cased it to `cmd_2` and dropped `mut`, breaking the build).

Addresses Copilot review on #3842.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Update tests/test_pivotp.rs by renaming the local variable `cmd2` to `cmd_2` and adjusting its usages (args, assert_err, output_stderr). This clarifies naming and keeps the test consistent with the surrounding code.
@jqnatividad jqnatividad merged commit 7ec7262 into master May 10, 2026
16 of 17 checks passed
@jqnatividad jqnatividad deleted the pivotp-quantile branch May 10, 2026 18:25
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.

3 participants