feat(pivotp): totals#3635
Merged
Merged
Conversation
#3631) Add support for appending grand total rows and inserting subtotal rows in pivot table output. Grand totals sum all numeric columns into a final row. Subtotals insert sum rows after each group change in the first index column (requires 2+ index columns). The --total-label flag allows customizing the label text (default: "Total"). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ions - Sort by first index column before computing subtotals to ensure contiguous groups regardless of upstream pivot ordering - Replace unwrap() with safe null handling in group column access - Fix misleading comment about grand total data source - Strengthen subtotal and combined tests to verify exact numeric values instead of only checking labels Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ering - Use `.ok().unwrap_or_default()` consistently when extracting group values from the column, matching the pattern used elsewhere in the function - Sort by all index columns (not just the first) in `insert_subtotals` to ensure deterministic intra-group ordering across Polars versions - Update test expectations to reflect alphabetical region ordering Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds optional grand totals and subtotals to the qsv pivotp command (Polars-backed pivot) to address feature request #3631.
Changes:
- Introduces
--grand-total,--subtotal, and--total-labelCLI flags for pivot output totals. - Implements total-row construction and subtotal row insertion into the pivot result DataFrame.
- Adds new integration tests covering grand totals, subtotals, and combined usage.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
src/cmd/pivotp.rs |
Adds CLI flags and implements grand total/subtotal row generation and insertion. |
tests/test_pivotp.rs |
Adds regression tests for the new totals/subtotals behavior and flags. |
- Use str() accessor instead of to_string()+trim_matches('"') for group values
- Propagate replace() error instead of silently discarding with let _
- Use vstack_mut for in-place stacking to avoid quadratic allocation
- Add validation guard for --grand-total with empty index columns
- Strengthen mean agg test to assert actual numeric grand total values
- Clarify doc comment on subtotal row ordering
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Fix doc comment that incorrectly said subtotals use existing row order when the function actually sorts by all index columns - Use str() accessor for boundary detection to match the group value extraction logic, ensuring consistent string representation Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…totals
Fix inconsistent null handling between current_val (Option) and start_val
(String via unwrap_or_default) in the str path of insert_subtotals. Two
consecutive null values were incorrectly treated as a group boundary because
None != Some("") evaluated to true. Now both use Option<String> so nulls
compare equal.
Also hoist group_col.str() before the loop to avoid redundant calls on
every iteration — the column type never changes.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
resolves #3631