SEPURI-SAI-KRISHNA commented on PR #42410:
URL: https://github.com/apache/superset/pull/42410#issuecomment-5113685402
Good catch, this one is real — fixed and pushed.
@rusackas heads-up, since you'd already approved: this is a third commit
fixing an
actual regression, not polish.
`exclude="label"` was being iterated character by character, so the guard
reported
`l`, `a`, `b`, `e`, `l` as missing and rejected a valid call:
```
Referenced columns not available in DataFrame: l, a, b, e, l
```
The scalar form is supported — `validate_column_args` normalises every column
argument through `scalar_to_sequence` to validate it, then passes the
*original*
value on, so the decorator accepts the string and hands the raw string
through.
Fixed by normalising with the same helper the decorator uses, so validation
and
execution agree on the shape:
```python
exclude = list(scalar_to_sequence(exclude))
```
Two corrections to the report, in both directions:
**It's older than the comment suggests.** The regression came in with the
first
commit on this branch, not the follow-up that added the column names. The
original
`any(column not in df_select.columns for column in exclude)` had exactly the
same
defect — the follow-up only made it visible by printing the characters. I
verified
by reverting to each form in turn; the new test fails against both:
```
commit 1 form -> Referenced columns not available in DataFrame.
commit 2 form -> Referenced columns not available in DataFrame: l, a, b, e, l
```
Before this branch, `df.drop("label", axis=1)` handled the scalar natively,
which
is what makes it a regression rather than a pre-existing limitation.
**But "Major" overstates the reach.** It isn't reachable through the
chart-data
API: `superset/charts/schemas.py:658` declares `exclude =
fields.List(fields.String())`,
so the payload is always a list. Only direct callers of the utility are
affected.
Worth fixing regardless — it's a behaviour regression against master and the
decorator's own contract.
`test_select_exclude_accepts_scalar` covers both the valid scalar and a
scalar
naming a column that doesn't exist. Verified with ruff 0.9.7, mypy 1.15.0,
pylint
3.3.7 (10.00/10); 290 passed across `pandas_postprocessing`, `queries` and
`common` (the two `test_prophet` failures are the optional `prophet` extra
missing
locally).
--
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]
