sadpandajoe commented on code in PR #44679: URL: https://github.com/apache/superset/pull/44679#discussion_r4300777535
########## docs/docs/using-superset/pivot-result-exports.md: ########## @@ -0,0 +1,58 @@ +<!-- +Licensed to the Apache Software Foundation (ASF) under one +or more contributor license agreements. See the NOTICE file +distributed with this work for additional information +regarding copyright ownership. The ASF licenses this file +to you under the Apache License, Version 2.0 (the +"License"); you may not use this file except in compliance +with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, +software distributed under the License is distributed on an +"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +KIND, either express or implied. See the License for the +specific language governing permissions and limitations +under the License. +--> + +# Pivot result aggregation exports + +Pivot Table result aggregation summarizes the grouped metric values returned by +the chart query. For example, with grouped values 1, 9, 20 and 100, choosing +**Median** produces a grand summary of 14.5. It does not calculate the median +of intermediate subtotals. Query filters and row limits apply before this step. + +## Configure the export + +Select a result aggregation in the Pivot Table chart, then save the chart for Review Comment: This tells analysts to select a result aggregation in the Pivot Table chart, but the chart's control panel on `master` only exposes `Show values as` and has no aggregation selector. A reader following this page cannot choose Median or Metric, so the page would describe behavior no one can reach until the frontend control lands. Should this page wait for that control, or say how the aggregation is set today? ########## superset/charts/client_processing.py: ########## @@ -598,7 +764,9 @@ def pivot_df( # pylint: disable=too-many-locals, too-many-arguments, too-many-s # convert to a MultiIndex to simplify logic if not isinstance(df.index, pd.MultiIndex): - df.index = pd.MultiIndex.from_tuples([(str(i),) for i in df.index]) + df.index = pd.MultiIndex.from_tuples( + [(i if independent_summaries else str(i),) for i in df.index] Review Comment: For a single row dimension this still stringifies labels in the main pass when the aggregate is `Metric` (or missing/unrecognised), while the AUTO-currency pass always keeps the raw labels because `independent_summaries` is true for `CURRENCY_CONTEXT_AGGREGATION`. `format_column` then does `currency_context.reindex(df.index)` against mismatched index types, so every row loses its per-row currency context. For example, rows `id` = 1, 2 with currencies USD/EUR, AUTO currency and `aggregateFunction: "Metric"` no longer format as `$`/`€` per row; before this change both passes used `str(i)`. Can both passes normalize the index the same way, with a `Metric` + AUTO currency + numeric-label test covering it? -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
