codeant-ai-for-open-source[bot] commented on code in PR #43203:
URL: https://github.com/apache/superset/pull/43203#discussion_r3788202325
##########
superset/utils/pandas_postprocessing/contribution.py:
##########
@@ -17,16 +17,56 @@
from __future__ import annotations
-from decimal import Decimal
from typing import Any
from flask_babel import gettext as _
from pandas import DataFrame, MultiIndex
+from pandas.api.types import infer_dtype, is_bool_dtype, is_numeric_dtype
from superset.exceptions import InvalidPostProcessingError
from superset.utils.core import PostProcessingContributionOrientation,
TIME_COMPARISON
from superset.utils.pandas_postprocessing.utils import validate_column_args
+# Inferred value types of an object-dtype column that the contribution
+# arithmetic can consume. `decimal` covers columns holding `decimal.Decimal`
+# (how drivers such as psycopg2 return NUMERIC/DECIMAL metrics); `empty`
+# covers an all-null column, which contributes nothing but is harmless once
+# the nulls are filled with zeros.
+_ARITHMETIC_OBJECT_DTYPES = frozenset({"decimal", "empty"})
+
+
+def _select_arithmetic_columns(df: DataFrame) -> DataFrame:
+ """
+ Select the columns whose values the contribution arithmetic can divide.
+
+ Numeric dtypes qualify directly. `decimal.Decimal` values -- how drivers
+ such as psycopg2 hand back NUMERIC/DECIMAL metrics -- live in an
+ object-dtype column, which ``select_dtypes`` cannot address: handing it
+ ``Decimal`` resolves to plain ``object`` and so selects every string,
+ dict and list column as well, leaving the division below to raise
+ ``TypeError`` on any result set that carries a non-numeric column. The
+ inferred value type separates Decimal columns from those, so the
+ remaining object columns are classified that way instead.
+
+ :param df: DataFrame to select columns from.
+ :return: Subset of `df` holding only the columns safe to divide.
+ """
+
+ # Booleans are excluded because ``is_numeric_dtype`` accepts them while
+ # ``select_dtypes(include=["number"])`` does not, and dividing them was
+ # never part of the calculation. Columns are addressed by position rather
+ # than by label so that duplicate labels stay distinguishable.
+ def is_arithmetic(position: int, dtype: Any) -> bool:
+ if is_numeric_dtype(dtype) and not is_bool_dtype(dtype):
+ return True
+ return infer_dtype(df.iloc[:, position], skipna=True) in (
+ _ARITHMETIC_OBJECT_DTYPES
+ )
Review Comment:
**Suggestion:** Decimal columns selected here are incompatible with the
production `contribution_totals` path: the query-context processor only puts
columns with dtype kinds in `biufc` into the totals dictionary, so object-dtype
Decimal metrics are absent. When totals are supplied, the later lookup receives
`None` and assigns zero instead of calculating the Decimal contribution. Either
include Decimal object columns when building totals or convert them to a
supported numeric representation before using the totals path. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Decimal metric percentages become zero with mixed metrics.
- ⚠️ Query-context totals omit object-dtype Decimal columns.
- ⚠️ Affected table and pie contribution visualizations.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=6053ce4d46c9437b87adce421efb799d&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=6053ce4d46c9437b87adce421efb799d&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/utils/pandas_postprocessing/contribution.py
**Line:** 62:64
**Comment:**
*Api Mismatch: Decimal columns selected here are incompatible with the
production `contribution_totals` path: the query-context processor only puts
columns with dtype kinds in `biufc` into the totals dictionary, so object-dtype
Decimal metrics are absent. When totals are supplied, the later lookup receives
`None` and assigns zero instead of calculating the Decimal contribution. Either
include Decimal object columns when building totals or convert them to a
supported numeric representation before using the totals path.
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%2F43203&comment_hash=94c6f39dea293335aad49371489bbd33eb06d9bbb0cf984189d9e5cee47d5661&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43203&comment_hash=94c6f39dea293335aad49371489bbd33eb06d9bbb0cf984189d9e5cee47d5661&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: [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]
