SEPURI-SAI-KRISHNA opened a new pull request, #43203:
URL: https://github.com/apache/superset/pull/43203
### SUMMARY
`contribution` post-processing crashes with a `TypeError` — surfacing to the
user
as an HTTP 500 — whenever the result set carries a non-numeric column, even
though its docstring states *"Non-numeric columns will be kept untouched."*
The cause is a single selector:
```python
numeric_df = contribution_df.select_dtypes(include=["number", Decimal])
```
`select_dtypes` resolves `Decimal` through `infer_dtype_from_object`, which
maps
any unrecognised Python type to `numpy.object_`. So `include=["number",
Decimal]`
is really `include=["number", "object"]`, and **every** string / dict / list
column is collected as if it were a metric. The division that follows then
blows
up:
```
TypeError: unsupported operand type(s) for /: 'str' and 'str' #
orientation=column
TypeError: unsupported operand type(s) for +: 'float' and 'str' #
orientation=row
```
Reproducing on `master`:
```python
>>> from pandas import DataFrame
>>> from superset.utils.pandas_postprocessing import contribution
>>> contribution(DataFrame({"label": ["x", "y"], "a": [1.0, 3.0]}))
TypeError: unsupported operand type(s) for /: 'str' and 'str'
```
The same defect silently disables the function's own validation guard.
Selecting
a string column is meant to raise `InvalidPostProcessingError` ("Column ...
is
not numeric"), but the check tests membership against the very list that now
wrongly contains string columns, so it never fires and the request 500s
instead
of returning a 400:
```python
>>> contribution(DataFrame({"label": ["x", "y"], "a": [1.0, 3.0]}),
columns=["label"])
TypeError: unsupported operand type(s) for /: 'str' and 'str' # expected:
InvalidPostProcessingError
```
The intent behind naming `Decimal` was sound — drivers such as psycopg2 hand
back `NUMERIC`/`DECIMAL` metrics as `decimal.Decimal`, which lands in an
object-dtype column — but a dtype selector cannot express "object columns
holding Decimals". This PR classifies the remaining object columns by their
inferred *value* type instead, which separates Decimal from strings, dicts
and
lists. Decimal metrics keep working; everything else is left untouched, as
documented.
Existing coverage missed this because `test_non_numeric_columns` uses
`__timestamp`, a `datetime64` column — already excluded correctly, since it
is
not object dtype. No test exercised the Decimal path at all.
Two incidental notes:
* Booleans are explicitly excluded so the selection stays byte-for-byte
equivalent to the previous `select_dtypes(include=["number"])` behaviour
(`is_numeric_dtype` accepts `bool`; `select_dtypes("number")` does not).
* The `fillna` is no longer `inplace`. The selection is a slice of
`contribution_df`, and an in-place fill on a slice raises
`SettingWithCopyWarning` under pandas 2 and is dropped outright under
copy-on-write.
A pre-existing `FutureWarning` ("Downcasting object dtype arrays on .fillna
...
is deprecated") is reproducible on `master` as well and is left alone here to
keep this change scoped to the crash.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — backend-only fix.
### TESTING INSTRUCTIONS
```bash
pytest tests/unit_tests/pandas_postprocessing/test_contribution.py
```
Six regression tests are added. Five of them fail on `master` with the exact
`TypeError`s above and pass with this change:
| test | asserts |
| --- | --- |
| `test_contribution_leaves_string_columns_untouched` | string column passes
through, metric still divided |
| `test_contribution_across_row_leaves_string_columns_untouched` | same for
`orientation=row` |
| `test_contribution_rejects_selected_string_column` | selecting a string
column raises `InvalidPostProcessingError` rather than 500ing |
| `test_contribution_on_decimal_columns` | `Decimal` metrics still
contribute (previously untested) |
| `test_contribution_ignores_columns_of_unsupported_objects` | dict-valued
column passes through |
| `test_contribution_keeps_all_null_object_columns` | pins the preserved
behaviour for an all-null object column |
The sixth passes either way by design — it pins behaviour deliberately left
unchanged, so that an all-`NULL` object column keeps behaving like an
all-`NaN`
float column.
Manually:
```python
from pandas import DataFrame
from decimal import Decimal
from superset.utils.pandas_postprocessing import contribution
# 500 on master, correct on this branch
contribution(DataFrame({"label": ["x", "y"], "a": [Decimal("1"),
Decimal("3")]}))
```
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
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]
