You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org> on 2026/10/15 03:14:17 UTC
Re: [PR] feat(pivot-table): add result aggregation to tabular exports [superset]
codeant-ai-for-open-source[bot] commented on code in PR #44679:
URL: https://github.com/apache/superset/pull/44679#discussion_r4278634448
##########
superset/charts/client_processing.py:
##########
@@ -479,6 +479,133 @@ 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 in {"First", "Last"}:
+ present = values.dropna()
+ return present.iloc[0 if name == "First" else -1] if len(present) else None
Review Comment:
**Suggestion:** For a group whose last result is NULL, this returns an earlier non-null value, while the browser returns NULL, so the tabular export disagrees.
**Assessment:** π `Major` Β· π `Occurrence: Sometimes` Β· π·οΈ `Api mismatch`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=088267417201437ebc71be754aea1779&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=088267417201437ebc71be754aea1779&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<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:** 494:495
**Comment:**
*Api Mismatch: For a group whose last result is NULL, this returns an earlier non-null value, while the browser returns NULL, so the tabular export disagrees.
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%2F44679&comment_hash=e3f1ec3c08f9c1feda4d812fe524b2720eb9d460a6e56c281eea69e18f8476c9&reaction=like'>π</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44679&comment_hash=e3f1ec3c08f9c1feda4d812fe524b2720eb9d460a6e56c281eea69e18f8476c9&reaction=dislike'>π</a>
##########
superset/charts/client_processing.py:
##########
@@ -479,6 +479,133 @@ 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 in {"First", "Last"}:
+ present = values.dropna()
+ return present.iloc[0 if name == "First" else -1] if len(present) else None
+ numeric = pd.to_numeric(values, errors="coerce").dropna()
Review Comment:
**Suggestion:** This drops NULL observations, but the browser converts them to zero for numeric reducers, so nullable exports show different averages, medians, extrema, and variances.
**Assessment:** π `Major` Β· π `Occurrence: Sometimes` Β· π·οΈ `Api mismatch`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=c0a4487da36f4d5eb709f88ed448ad2c&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=c0a4487da36f4d5eb709f88ed448ad2c&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<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:** 496:496
**Comment:**
*Api Mismatch: This drops NULL observations, but the browser converts them to zero for numeric reducers, so nullable exports show different averages, medians, extrema, and variances.
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%2F44679&comment_hash=fac206bf839cb7d94b69148110c083d4ac9ff91ecd89462da1cf8091c5278acc&reaction=like'>π</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44679&comment_hash=fac206bf839cb7d94b69148110c083d4ac9ff91ecd89462da1cf8091c5278acc&reaction=dislike'>π</a>
##########
superset/charts/client_processing.py:
##########
@@ -978,6 +1162,7 @@ def build_pivot_currency_context(
currency_pivot_options = {
**pivot_options,
"aggfunc": CURRENCY_CONTEXT_AGGREGATION,
+ "result_aggregation": False,
Review Comment:
**Suggestion:** With subtotals enabled but grand totals disabled, this prevents the currency-context pivot from creating subtotal labels, so AUTO formatting loses each subtotalβs currency context.
**Assessment:** π `Major` Β· π `Occurrence: Rarely` Β· π·οΈ `Incomplete implementation`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=33e1894fde3e4a43ae5985b4356e9cbb&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=33e1894fde3e4a43ae5985b4356e9cbb&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<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:** 1165:1165
**Comment:**
*Incomplete Implementation: With subtotals enabled but grand totals disabled, this prevents the currency-context pivot from creating subtotal labels, so AUTO formatting loses each subtotalβs currency context.
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%2F44679&comment_hash=0dd639600a8da830501bb93739afdb54e44a97f10a689d84fc2bd2037dd8b8e6&reaction=like'>π</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44679&comment_hash=0dd639600a8da830501bb93739afdb54e44a97f10a689d84fc2bd2037dd8b8e6&reaction=dislike'>π</a>
##########
superset/charts/client_processing.py:
##########
@@ -479,6 +479,133 @@ 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 in {"First", "Last"}:
+ present = values.dropna()
+ return present.iloc[0 if name == "First" else -1] 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 == "Sample Variance":
+ return numeric.var(ddof=1) if len(numeric) > 1 else 0
+ if name == "Sample Standard Deviation":
+ return numeric.std(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[str, ...], pd.DataFrame]:
+ """Index contributing records by the dimension prefix of a summary scope."""
+ if not dimensions:
+ return {(): source}
+ keys = [source[dim].fillna("SUPERSET_PANDAS_NAN").astype(str) 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 _apply_result_aggregation(
+ 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,
+ show_values_as: Optional[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
+
+ grouped_sources: dict[tuple[int, int], dict[tuple[str, ...], pd.DataFrame]] = {}
+
+ def contributing(
+ row: Any,
+ column: Any,
+ row_depth: int,
+ column_depth: int,
+ selected_metrics: list[str],
+ ) -> pd.Series:
+ level = (row_depth, column_depth)
+ if level not in grouped_sources:
+ grouped_sources[level] = _group_result_records(
+ source, rows[:row_depth] + columns[:column_depth]
+ )
+ key = _rollup_key(row, row_depth, metric_level, is_column=False) + _rollup_key(
+ column,
+ column_depth,
+ metric_level,
+ is_column=True,
+ )
+ selected = grouped_sources[level].get(key, source.iloc[:0])
+ # Row-major order matches the browser's metric unpivoting.
+ return pd.Series(selected[selected_metrics].to_numpy().ravel(), dtype=object)
+
+ old_fraction = aggregate.partition(" as Fraction of ")[2]
+ fraction = {
+ "Total": "percent_total",
+ "Rows": "percent_row",
+ "Columns": "percent_col",
+ }.get(old_fraction) or show_values_as
+ 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))
+ metric = _metric_of_column(column, metric_level)
+ selected_metrics = (
+ [metric] if metric in metrics else list(dict.fromkeys(metrics))
+ )
+ # Match the browser's mixed-metric guard: an axis/corner that
+ # combines different metrics has no meaningful single summary.
+ if len(selected_metrics) != 1:
+ frame.iloc[ri, ci] = None
+ continue
+ value = _reduce_result_values(
+ contributing(row, column, rd, cd, selected_metrics), aggregate
+ )
+ if fraction in SHOW_VALUES_AS_PERCENT_MODES:
+ dr, dc = rd, cd
+ if fraction == "percent_total":
+ dr, dc = 0, 0
+ elif (fraction == "percent_row") != metrics_on_rows:
+ dc = 0
+ else:
+ dr = 0
+ denominator = _reduce_result_values(
+ contributing(row, column, dr, dc, selected_metrics), aggregate
+ )
+ value = (
+ value / denominator if value is not None and denominator else None
+ )
Review Comment:
**Suggestion:** When both options are set, this applies `showValuesAs` after the result reducer, while the browser gives the result reducer precedence, so chart and export values differ.
**Assessment:** π `Major` Β· π `Occurrence: Sometimes` Β· π·οΈ `Api mismatch`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=438db625ff824af3bc348ed58a28af1b&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=438db625ff824af3bc348ed58a28af1b&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<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:** 591:604
**Comment:**
*Api Mismatch: When both options are set, this applies `showValuesAs` after the result reducer, while the browser gives the result reducer precedence, so chart and export values differ.
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%2F44679&comment_hash=3ca9aea82dd0d2c9ce8e4ecf08be1d07a7074d1a01e2946c1539125e39309842&reaction=like'>π</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44679&comment_hash=3ca9aea82dd0d2c9ce8e4ecf08be1d07a7074d1a01e2946c1539125e39309842&reaction=dislike'>π</a>
##########
superset/charts/client_processing.py:
##########
@@ -479,6 +479,133 @@ 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 in {"First", "Last"}:
+ present = values.dropna()
+ return present.iloc[0 if name == "First" else -1] 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 == "Sample Variance":
+ return numeric.var(ddof=1) if len(numeric) > 1 else 0
+ if name == "Sample Standard Deviation":
+ return numeric.std(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[str, ...], pd.DataFrame]:
+ """Index contributing records by the dimension prefix of a summary scope."""
+ if not dimensions:
+ return {(): source}
+ keys = [source[dim].fillna("SUPERSET_PANDAS_NAN").astype(str) for dim in dimensions]
+ return {
+ key if isinstance(key, tuple) else (key,): group
+ for key, group in source.groupby(keys, sort=False, dropna=False)
+ }
Review Comment:
**Suggestion:** Converting dimension keys to strings merges distinct values such as integer `1` and string `1`, so summaries for those groups include each otherβs records.
**Assessment:** π `Major` Β· π `Occurrence: Rarely` Β· π·οΈ `Type error`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=bd3330af87fa4da4be5c252872fda410&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=bd3330af87fa4da4be5c252872fda410&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<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:** 520:524
**Comment:**
*Type Error: Converting dimension keys to strings merges distinct values such as integer `1` and string `1`, so summaries for those groups include each otherβs records.
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%2F44679&comment_hash=05e58cd050b0c148a42e65cd5c33d83e989f086df8bfc2b43ab6f2f6fe658005&reaction=like'>π</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44679&comment_hash=05e58cd050b0c148a42e65cd5c33d83e989f086df8bfc2b43ab6f2f6fe658005&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