SEPURI-SAI-KRISHNA commented on code in PR #43203:
URL: https://github.com/apache/superset/pull/43203#discussion_r3788722901
##########
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:
Confirmed the mechanism, query_context_processor.py:446 builds totals with
df[col].dtype.kind in "biufc", which excludes object-dtype Decimal columns, so
the
contribution_totals.get(col) lookup returns None and assigns zero.
It isn't introduced here, though. The previous
select_dtypes(include=["number", Decimal]) resolved Decimal to object and so
selected the same Decimal columns; the set reaching the
totals path is unchanged by this PR. The fix belongs in the totals builder,
which should include object columns whose inferred type is decimal. Happy to
open that as a follow-up, it's a real bug, just a different one.
--
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]
