fitzee commented on code in PR #44679:
URL: https://github.com/apache/superset/pull/44679#discussion_r4308842491
##########
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:
Fixed in 70bf9ed512d34b4e1df432b20f343b80a40fb8e7. The currency-context pass
now follows the main pass's index normalization: default/Metric/unrecognized
settings retain string labels, while explicit result aggregations retain typed
labels in both passes.
Regression test: `test_auto_currency_index_matches_metric_values`. Numeric
and datetime labels are covered with unset, Metric, unknown and Sum settings,
using a GBP fallback to expose lost USD/EUR row context. The numeric
default/Metric/unknown cases fail before the fix and pass after it.
Siblings checked:
- `superset/charts/client_processing.py` — fixed in
70bf9ed512d34b4e1df432b20f343b80a40fb8e7: main pivot and currency-context
pivot, legacy/default modes and result mode.
Full chart suite: 450 passed, 2 expected failures. Pre-commit passed. CI at
this head remains pending.
##########
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:
Corrected the user guide in 70bf9ed512d34b4e1df432b20f343b80a40fb8e7. It now
explicitly says the chart editor has Show values as but no result-aggregation
selector, and explains that this export behavior reads an existing saved
aggregateFunction setting. Removed the instruction to select Median/Metric in
the editor and updated the PR description to make the same boundary clear. This
change does not add a frontend control.
##########
superset/charts/client_processing.py:
##########
@@ -479,6 +479,151 @@ def _apply_show_values_as( # pylint:
disable=too-many-arguments
return numeric / denominator.replace(0, np.nan)
+def _reduce_result_values(values: pd.Series, aggregate: str) -> Any:
+ """Reduce original metric results without aggregating intermediate
summaries."""
+ if values.empty:
+ return None
+ name = aggregate.split(" as Fraction of ")[0]
+ if name == "Count":
+ return len(values)
+ if name == "Count Unique Values":
+ return values.nunique(dropna=False)
+ if name == "List Unique Values":
+ return ", ".join(sorted({str(value) for value in values}))
+ if name == "Last":
+ return values.iloc[-1]
+ if name == "First":
+ present = values.dropna()
+ return present.iloc[0] if len(present) else None
+ numeric = pd.to_numeric(values, errors="coerce").dropna()
+ if name == "Sum":
+ text = values.dropna()
+ return text.iloc[-1] if numeric.empty and len(text) else numeric.sum()
+ if numeric.empty:
+ return None
+ if name in {"Sample Variance", "Sample Standard Deviation"}:
+ statistic = {
+ "Sample Variance": numeric.var,
+ "Sample Standard Deviation": numeric.std,
+ }
+ return statistic[name](ddof=1) if len(numeric) > 1 else 0
+ return {
+ "Average": numeric.mean,
+ "Median": numeric.median,
+ "Minimum": numeric.min,
+ "Maximum": numeric.max,
+ }[name]()
+
+
+def _group_result_records(
+ source: pd.DataFrame, dimensions: list[str]
+) -> dict[tuple[Any, ...], pd.DataFrame]:
+ """Index contributing records by the dimension prefix of a summary
scope."""
+ if not dimensions:
+ return {(): source}
+ keys = [
+ source[dim].astype(object).fillna("SUPERSET_PANDAS_NAN") for dim in
dimensions
+ ]
+ return {
+ key if isinstance(key, tuple) else (key,): group
+ for key, group in source.groupby(keys, sort=False, dropna=False)
+ }
+
+
+def _result_scope_key(
+ row: Any, column: Any, row_depth: int, column_depth: int, metric_level: int
+) -> tuple[Any, ...]:
+ """Keep typed dimensions consistent between source groups and pivot
labels."""
+ # Series.astype(str) formats dates differently from scalar str(), and
merges
+ # numeric and text labels. Result scopes must preserve the original
scalars.
+ row_parts = row if isinstance(row, tuple) else (row,)
+ column_parts = column if isinstance(column, tuple) else (column,)
+ dimensions = tuple(
+ part for index, part in enumerate(column_parts) if index !=
metric_level
+ )
+ return row_parts[:row_depth] + dimensions[:column_depth]
+
+
+def _apply_result_aggregation( # noqa: C901
+ frame: pd.DataFrame,
+ source: pd.DataFrame,
+ rows: list[str],
+ columns: list[str],
+ metrics: list[str],
+ metric_level: int,
+ row_depths: dict[Any, int],
+ column_depths: dict[Any, int],
+ aggregate: str,
+ metrics_on_rows: bool,
+) -> pd.DataFrame:
+ """Calculate every scope from its original contributing query results."""
+
+ if not set(metrics).issubset(source.columns):
+ return frame
+
+ @lru_cache(maxsize=None)
+ def grouped_source(
+ row_depth: int, column_depth: int
+ ) -> dict[tuple[Any, ...], pd.DataFrame]:
+ return _group_result_records(source, rows[:row_depth] +
columns[:column_depth])
+
+ reduced: dict[tuple[Any, ...], Any] = {}
+
+ def reduce_scope(
+ row: Any,
+ column: Any,
+ row_depth: int,
+ column_depth: int,
+ selected_metrics: list[str],
+ ) -> Any:
+ level = (row_depth, column_depth)
+ key = _result_scope_key(row, column, row_depth, column_depth,
metric_level)
+ cache_key = (level, key, tuple(selected_metrics))
+ if cache_key not in reduced:
+ selected = grouped_source(*level).get(key, source.iloc[:0])
+ # Row-major order matches the browser's metric unpivoting.
+ values = pd.Series(
+ selected[selected_metrics].to_numpy().ravel(), dtype=object
+ )
+ reduced[cache_key] = _reduce_result_values(values, aggregate)
+ return reduced[cache_key]
+
+ old_fraction = aggregate.partition(" as Fraction of ")[2]
+ fraction = {
+ "Total": "percent_total",
+ "Rows": "percent_row",
+ "Columns": "percent_col",
+ }.get(old_fraction)
+ for ri, row in enumerate(frame.index):
+ for ci, column in enumerate(frame.columns):
+ rd = row_depths.get(row, len(rows))
+ cd = column_depths.get(column, len(columns))
Review Comment:
Fixed in 70bf9ed512d34b4e1df432b20f343b80a40fb8e7. Summary depth lookups
normalize NaN/NaT padding back to empty strings. The same normalization is used
when filtering summary visibility; otherwise a disabled grand total could
remain visible because its typed label also missed that lookup.
Regression test: `test_datetime_result_summary_depth`. Covers metrics on
rows/columns, combined/uncombined metrics and totals enabled/disabled. Four
cases fail against the previous implementation and pass with the fix.
Siblings checked:
- `superset/charts/client_processing.py` — fixed in
70bf9ed512d34b4e1df432b20f343b80a40fb8e7: row/column depth lookup and
row/column visibility filtering.
Full chart suite: 450 passed, 2 expected failures. Pre-commit passed. CI at
this head remains pending.
--
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]
