You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "divyeshreddy02 (via GitHub)" <gi...@apache.org> on 2026/05/29 06:17:03 UTC

[PR] fix(pivot-table): Rename 'Subtotal' to 'Subvalue' with aggregation fu… [superset]

divyeshreddy02 opened a new pull request, #40513:
URL: https://github.com/apache/superset/pull/40513

   …nction
   
   The Pivot Table visualization previously used 'Subtotal' as a label for subgroup aggregations regardless of which aggregation function was used (e.g., Sum, Average, Max). This was confusing because 'Subtotal' implies a specific 'Sum' operation.
   
   This change:
   - Renames 'Subtotal' to 'Subvalue (%(aggfunc)s)'
   - Includes the actual aggregation function name (Sum, Average, etc.)
   - Makes the label consistent with 'Total (Sum)' at the top level
   
   Fixes #35089
   
   <!---
   Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
   Example:
   fix(dashboard): load charts correctly
   -->
   
   ### SUMMARY
   <!--- Describe the change below, including rationale and design decisions -->
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   <!--- Skip this if not applicable -->
   
   ### TESTING INSTRUCTIONS
   <!--- Required! What steps can be taken to manually verify the changes? -->
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [ ] Has associated issue:
   - [ ] Required feature flags:
   - [ ] Changes UI
   - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
     - [ ] Migration is atomic, supports rollback & is backwards-compatible
     - [ ] Confirm DB migration upgrade and downgrade tested
     - [ ] Runtime estimates and downtime expectations provided
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(pivot-table): Rename 'Subtotal' to 'Subvalue' with aggregation fu… [superset]

Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #40513:
URL: https://github.com/apache/superset/pull/40513#issuecomment-4605809861

   ## [Codecov](https://app.codecov.io/gh/apache/superset/pull/40513?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :x: Patch coverage is `0%` with `7 lines` in your changes missing coverage. Please review.
   :white_check_mark: Project coverage is 55.28%. Comparing base ([`b0da0cf`](https://app.codecov.io/gh/apache/superset/commit/b0da0cf202b277923e642efdfb92716527364de9?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`ea02f11`](https://app.codecov.io/gh/apache/superset/commit/ea02f111a380c27fbff1f544bdd2a0b63767baf8?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   :warning: Report is 103 commits behind head on master.
   
   | [Files with missing lines](https://app.codecov.io/gh/apache/superset/pull/40513?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
   |---|---|---|
   | [superset/commands/streaming\_export/base.py](https://app.codecov.io/gh/apache/superset/pull/40513?src=pr&el=tree&filepath=superset%2Fcommands%2Fstreaming_export%2Fbase.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvY29tbWFuZHMvc3RyZWFtaW5nX2V4cG9ydC9iYXNlLnB5) | 0.00% | [5 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/40513?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [superset/charts/client\_processing.py](https://app.codecov.io/gh/apache/superset/pull/40513?src=pr&el=tree&filepath=superset%2Fcharts%2Fclient_processing.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvY2hhcnRzL2NsaWVudF9wcm9jZXNzaW5nLnB5) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/40513?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #40513      +/-   ##
   ==========================================
   - Coverage   64.07%   55.28%   -8.80%     
   ==========================================
     Files        2592     2592              
     Lines      139578   139582       +4     
     Branches    32431    32432       +1     
   ==========================================
   - Hits        89441    77167   -12274     
   - Misses      48599    61748   +13149     
   + Partials     1538      667     -871     
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/superset/pull/40513/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [hive](https://app.codecov.io/gh/apache/superset/pull/40513/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `39.09% <0.00%> (-0.01%)` | :arrow_down: |
   | [mysql](https://app.codecov.io/gh/apache/superset/pull/40513/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [postgres](https://app.codecov.io/gh/apache/superset/pull/40513/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [presto](https://app.codecov.io/gh/apache/superset/pull/40513/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `40.76% <0.00%> (-0.01%)` | :arrow_down: |
   | [python](https://app.codecov.io/gh/apache/superset/pull/40513/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `40.83% <0.00%> (-19.31%)` | :arrow_down: |
   | [sqlite](https://app.codecov.io/gh/apache/superset/pull/40513/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [unit](https://app.codecov.io/gh/apache/superset/pull/40513/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   </details>
   
   [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/superset/pull/40513?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
   - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </details>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(pivot-table): Rename 'Subtotal' to 'Subvalue' with aggregation fu… [superset]

Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.
sadpandajoe commented on code in PR #40513:
URL: https://github.com/apache/superset/pull/40513#discussion_r3344666958


##########
superset/commands/streaming_export/base.py:
##########


Review Comment:
   Did we mean to update this? I don't see how this code has to do with subtotal name change.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(pivot-table): Rename 'Subtotal' to 'Subvalue' with aggregation fu… [superset]

Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #40513:
URL: https://github.com/apache/superset/pull/40513#discussion_r3322529455


##########
superset/charts/client_processing.py:
##########
@@ -175,7 +175,7 @@ def pivot_df(  # pylint: disable=too-many-locals, too-many-arguments, too-many-s
                 slice_ = df.columns.get_loc(subgroup)
                 subtotal = pivot_v2_aggfunc_map[aggfunc](df.iloc[:, slice_], axis=1)
                 depth = df.columns.nlevels - len(subgroup) - 1
-                total = metric_name if level == 0 else __("Subtotal")
+                total = metric_name if level == 0 else __("Subvalue (%(aggfunc)s)", aggfunc=aggfunc)

Review Comment:
   **Suggestion:** This new subtotal label diverges from the frontend pivot-table renderer, which still renders subgroup headers as `Subtotal`. Since this module is meant to reproduce client-side processing for reports, changing only the backend label creates a contract mismatch (Explore UI vs CSV/JSON/report output labels differ). Keep both sides aligned by updating the frontend subtotal label logic in the same change or using the same label here. [api mismatch]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ⚠️ Pivot table CSV subtotals differ from Explore UI.
   - ⚠️ Scheduled pivot_table_v2 reports show renamed subtotal labels.
   - ⚠️ Downstream tooling relying on UI labels may misalign.
   ```
   </details>
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. Note that `superset/charts/client_processing.py:18-25` documents this module as
   reproducing client-side post-processing so that reports show the same data as Explore
   (pivot table UI).
   
   2. On the frontend, `pivot_table_v2` uses the pivot-table plugin wired via
   `transformProps` at
   `superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/transformProps.ts:98-120`
   and `PivotTableChart` (see plugin wiring); it passes subtotal controls
   (`rowSubtotalPosition`, `colSubtotalPosition`, etc.) but does not receive or override the
   backend subtotal label, so the UI continues to use the pivot library's built‑in subtotal
   labeling (currently "Subtotal").
   
   3. For reports/exports, the backend entrypoint `apply_client_processing()` at
   `superset/charts/client_processing.py:13-27` routes `viz_type == "pivot_table_v2"` through
   `pivot_table_v2()`, which in turn calls `pivot_df()` with
   `aggfunc=form_data["aggregateFunction"]` to recompute the pivot for CSV/JSON (see tests
   `test_apply_client_processing_json_format` and `test_apply_client_processing_csv_format`
   at `tests/unit_tests/charts/test_client_processing.py:1193-1379`).
   
   4. Inside `pivot_df()`, subtotals are now labeled with `__("Subvalue (%(aggfunc)s)",
   aggfunc=aggfunc)` for non-top levels (`superset/charts/client_processing.py:77` for
   `metric_name` and lines `119` and `26` for `total`), so for `aggfunc="Sum"` the exported
   CSV/JSON indices contain "Subvalue (Sum)" while the Explore UI subtotal headers remain
   "Subtotal"; this breaks the intended parity between Explore and exported reports for
   subtotal labels.
   ```
   </details>
   
   [Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=d7f32be7a2234d84943a4184b7a56c55&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=d7f32be7a2234d84943a4184b7a56c55&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
   
   *(Use Cmd/Ctrl + Click for best experience)*
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** superset/charts/client_processing.py
   **Line:** 178:178
   **Comment:**
   	*Api Mismatch: This new subtotal label diverges from the frontend pivot-table renderer, which still renders subgroup headers as `Subtotal`. Since this module is meant to reproduce client-side processing for reports, changing only the backend label creates a contract mismatch (Explore UI vs CSV/JSON/report output labels differ). Keep both sides aligned by updating the frontend subtotal label logic in the same change or using the same label here.
   
   Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
   Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
   ```
   </details>
   <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40513&comment_hash=47cefaf83167a7455617770da1586f45dda8d93bc88b87add33419e583284422&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40513&comment_hash=47cefaf83167a7455617770da1586f45dda8d93bc88b87add33419e583284422&reaction=dislike'>👎</a>



##########
tests/unit_tests/charts/test_client_processing.py:
##########
@@ -371,10 +371,10 @@ def test_pivot_df_single_row_two_metrics():
 |:-------------------------|-------------------:|
 | ('SUM(num)', 'boy')      |              47123 |
 | ('SUM(num)', 'girl')     |             118065 |
-| ('SUM(num)', 'Subtotal') |             165188 |
+| ('SUM(num)', 'Subvalue') |             165188 |
 | ('MAX(num)', 'boy')      |               1280 |
 | ('MAX(num)', 'girl')     |               2588 |
-| ('MAX(num)', 'Subtotal') |               3868 |
+| ('MAX(num)', 'Subvalue') |               3868 |

Review Comment:
   **Suggestion:** The updated expectations still assert plain `Subvalue`, but the new implementation now generates `Subvalue (<aggfunc>)` (for example, `Subvalue (Sum)`). These snapshots will fail against the new behavior and make the test suite unreliable; update expected markdown labels to include the aggregation suffix consistently. [logic error]
   
   <details>
   <summary><b>Severity Level:</b> Critical 🚨</summary>
   
   ```mdx
   - ❌ Pivot table backend unit tests fail on subtotal expectations.
   - ❌ CI for charts client_processing module will be red.
   - ⚠️ Future regressions in subtotal labeling harder to detect.
   ```
   </details>
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. Run the unit test `test_pivot_df_single_row_two_metrics` defined at
   `tests/unit_tests/charts/test_client_processing.py:361-381`, which builds a small
   DataFrame and calls `pivot_df(...)` with `aggfunc="Sum"`, `show_rows_total=True`,
   `show_columns_total=True`, and `apply_metrics_on_rows=True`.
   
   2. The test asserts on `pivoted.to_markdown()` including subtotal rows labeled
   `('SUM(num)', 'Subvalue')` and `('MAX(num)', 'Subvalue')` (lines 373-378), i.e. without
   any aggregation suffix.
   
   3. At runtime, `pivot_df()` in `superset/charts/client_processing.py` now computes
   `metric_name = __("Total (%(aggfunc)s)", aggfunc=aggfunc)` at line 77 and uses `total =
   metric_name if level == 0 else __("Subvalue (%(aggfunc)s)", aggfunc=aggfunc)` when
   inserting subtotal columns/rows at lines 119 and 26, so with `aggfunc="Sum"` the subtotal
   label becomes `"Subvalue (Sum)"`, not `"Subvalue"`.
   
   4. As a result, the MultiIndex entries emitted by `pivot_df()` for the subtotal rows
   contain `"Subvalue (Sum)"`, making the `to_markdown()` output diverge from the hard-coded
   expectations in `test_pivot_df_single_row_two_metrics`, causing this and other similar
   tests (e.g., expectations at `tests/unit_tests/charts/test_client_processing.py:402-405`,
   `543-546`, `571-574`, `1075-1085`, etc.) to fail consistently until the snapshots are
   updated to include the aggregation suffix.
   ```
   </details>
   
   [Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=8c457f0143b146adb29b0e24a3cac58e&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=8c457f0143b146adb29b0e24a3cac58e&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
   
   *(Use Cmd/Ctrl + Click for best experience)*
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** tests/unit_tests/charts/test_client_processing.py
   **Line:** 374:377
   **Comment:**
   	*Logic Error: The updated expectations still assert plain `Subvalue`, but the new implementation now generates `Subvalue (<aggfunc>)` (for example, `Subvalue (Sum)`). These snapshots will fail against the new behavior and make the test suite unreliable; update expected markdown labels to include the aggregation suffix consistently.
   
   Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
   Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
   ```
   </details>
   <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40513&comment_hash=9126f7c4693d14fd64ea1daa63bdeee42f2745552fe4e2d67779050a71ee339c&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40513&comment_hash=9126f7c4693d14fd64ea1daa63bdeee42f2745552fe4e2d67779050a71ee339c&reaction=dislike'>👎</a>



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(pivot-table): Rename 'Subtotal' to 'Subvalue' with aggregation fu… [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40513:
URL: https://github.com/apache/superset/pull/40513#issuecomment-4571409308

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(pivot-table): Rename 'Subtotal' to 'Subvalue' with aggregation fu… [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40513:
URL: https://github.com/apache/superset/pull/40513#issuecomment-4573609084

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
   
   <details>
   <summary>
   Bito Usage Guide
   </summary>
   
   **Commands**
         
   Type the following command in the pull request comment and save the comment.
         
   - `/review` - Manually triggers a full AI review.
   
   - `/pause` - Pauses automatic reviews on this pull request.
   - `/resume` - Resumes automatic reviews.
   - `/resolve` - Marks all Bito-posted review comments as resolved.
   - `/abort` - Cancels all in-progress reviews.
   
         
   Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
         
   **Configuration**
   
   This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
         
   **Documentation & Help**
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
   
   </details>
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(pivot-table): Rename 'Subtotal' to 'Subvalue' with aggregation fu… [superset]

Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.
sadpandajoe commented on code in PR #40513:
URL: https://github.com/apache/superset/pull/40513#discussion_r3344566318


##########
superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/TableRenderers.tsx:
##########
@@ -1333,7 +1333,9 @@ export function TableRenderer(props: TableRendererProps) {
               true,
             )}
           >
-            {t('Subtotal')}
+            {t('Subvalue (%(aggregatorName)s)', {

Review Comment:
   From https://github.com/apache/superset/issues/35089#issuecomment-3802636253 it should be "Summary" and not "Subvalue".



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(pivot-table): Rename 'Subtotal' to 'Subvalue' with aggregation fu… [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #40513:
URL: https://github.com/apache/superset/pull/40513#issuecomment-5039888138

   Thanks @divyeshreddy02! Unfortunately #41184 (SIP-216) removes the Aggregation function control entirely, so there won't be an aggfunc to show in the label once it lands. Closing this to avoid conflict churn, but you're more than welcome to review/test #41184, or take a pass at the labels in a follow-up after it merges.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(pivot-table): Rename 'Subtotal' to 'Subvalue' with aggregation fu… [superset]

Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.
sadpandajoe commented on code in PR #40513:
URL: https://github.com/apache/superset/pull/40513#discussion_r3344688061


##########
superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/TableRenderers.tsx:
##########


Review Comment:
   Should this also be renamed? If so we should do a scan on what other areas we may have missed.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(pivot-table): Rename 'Subtotal' to 'Subvalue' with aggregation fu… [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #40513:
URL: https://github.com/apache/superset/pull/40513#issuecomment-4571409324

   Congrats on making your first PR and thank you for contributing to Superset! :tada: :heart:
   
   Please read our [New Contributor Welcome & Expectations](https://github.com/apache/superset/wiki/New-Contributor-Welcome-&-Expectations) guide.
   
   We hope to see you in our [Slack](https://apache-superset.slack.com/) community too! Not signed up? Use our [Slack App](http://bit.ly/join-superset-slack) to self-register.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(pivot-table): Rename 'Subtotal' to 'Subvalue' with aggregation fu… [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas closed pull request #40513: fix(pivot-table): Rename 'Subtotal' to 'Subvalue' with aggregation fu…
URL: https://github.com/apache/superset/pull/40513


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org