SEPURI-SAI-KRISHNA commented on code in PR #43225:
URL: https://github.com/apache/superset/pull/43225#discussion_r3998628815
##########
superset/charts/schemas.py:
##########
@@ -611,17 +613,18 @@ class
ChartDataRollingOptionsSchema(ChartDataPostProcessingOperationOptionsSchem
Rolling operation config.
"""
- columns = (
- fields.Dict(
- metadata={
- "description": "columns on which to perform rolling, mapping
source "
- "column to target column. For instance, `{'y': 'y'}` will
replace the "
- "column `y` with the rolling value in `y`, while `{'y': 'y2'}`
will add " # noqa: E501
- "a column `y2` based on rolling values calculated from `y`,
leaving the " # noqa: E501
- "original column `y` unchanged.",
- "example": {"weekly_rolling_sales": "sales"},
- },
- ),
+ columns = fields.Dict(
+ metadata={
+ "description": "columns on which to perform rolling, mapping
source "
+ "column to target column. For instance, `{'y': 'y'}` will replace
the "
+ "column `y` with the rolling value in `y`, while `{'y': 'y2'}`
will add "
+ "a column `y2` based on rolling values calculated from `y`,
leaving the "
+ "original column `y` unchanged.",
+ "example": {"weekly_rolling_sales": "sales"},
Review Comment:
You are right, and it is fixed in `b46dada0`.
`rolling()` selects the source frame with `df.loc[:, columns.keys()]`
(`superset/utils/pandas_postprocessing/rolling.py:69`), so the keys are the
columns
that must already exist. The example asked for a `weekly_rolling_sales`
source, which
no client would have, and it contradicted the field's own description one
line above.
Copying it verbatim fails before the target column is ever created.
Changed to `{"sales": "weekly_rolling_sales"}` and regenerated the spec;
that is the
whole diff of the new commit.
Worth noting this example is older than the PR. It could not mislead anyone
until now
because the trailing comma kept `columns` out of the published schema
entirely --
master's `ChartDataRollingOptionsSchema` has no `columns` property at all.
Restoring
the field is what would have exposed the reversed example to clients, so
correcting it
belongs in this change rather than a follow-up.
--
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]
