aminghadersohi commented on code in PR #42070:
URL: https://github.com/apache/superset/pull/42070#discussion_r3599530943
##########
superset/mcp_service/chart/schemas.py:
##########
@@ -2009,6 +2009,105 @@ def validate_percentiles_and_dimensions(self) ->
"BoxPlotChartConfig":
return self
+class WaterfallChartConfig(UnknownFieldCheckMixin):
+ """Config for waterfall charts (viz_type ``waterfall``)."""
+
+ model_config = ConfigDict(extra="ignore", populate_by_name=True)
+
+ chart_type: Literal["waterfall"] = "waterfall"
+ x_axis: ColumnRef = Field(
+ ...,
+ description="Category or period column along the x-axis (often "
+ "temporal, e.g. month); each value is one waterfall step",
+ )
+ metric: ColumnRef = Field(
+ ...,
+ description="Metric whose per-step change is plotted (use aggregate "
+ "e.g. SUM for ad-hoc, or saved_metric=True for a saved metric)",
+ )
+ breakdown: ColumnRef | None = Field(
+ None,
+ validation_alias=AliasChoices("breakdown", "groupby"),
+ description="Optional single category column that breaks each "
+ "x-axis period into per-category steps (form_data 'groupby'; the "
+ "frontend Breakdowns control is single-select)",
+ )
+ time_grain: TimeGrain | None = Field(
+ None,
+ description="Time bucket for a temporal x_axis (PT1H, P1D, P1W, "
+ "P1M, P1Y); each bucket becomes one waterfall step. Ignored for a "
+ "non-temporal x_axis.",
+ validation_alias=AliasChoices("time_grain", "time_grain_sqla"),
+ )
+ show_total: bool = Field(
+ True, description="Append a total bar per period (frontend default)"
+ )
+ show_legend: bool = Field(
+ False, description="Show the legend (frontend default: off)"
+ )
+ increase_label: str = Field("Increase", max_length=50)
+ decrease_label: str = Field("Decrease", max_length=50)
+ total_label: str = Field("Total", max_length=50)
+ x_axis_time_format: str = Field(
+ "smart_date",
+ description="Time format for a temporal x-axis (e.g. 'smart_date',
'%Y-%m-%d')",
+ max_length=50,
+ )
+ y_axis_format: str = Field("SMART_NUMBER", max_length=50)
+ currency_format: CurrencyFormat | None = Field(
+ None,
+ description="Currency symbol applied to the metric value",
+ )
+ filters: List[FilterConfig] | None = Field(
+ None,
+ description="Structured filters (column/op/value). "
+ "Do NOT use adhoc_filters or raw SQL expressions.",
+ )
+ row_limit: int = Field(
+ 10000,
+ description="Max grouped rows (frontend shared default)",
+ ge=1,
+ le=50000,
+ )
+
+ @model_validator(mode="before")
+ @classmethod
+ def unwrap_list_breakdown(cls, data: Any) -> Any:
+ """Accept the native form_data shape where ``groupby`` is a list.
+
+ The frontend Breakdowns control is single-select but stores its value
+ as a one-item list (e.g. ``["region"]``), so a config round-tripped
+ from existing waterfall form_data sends a list. Unwrap a length-1
+ list to the single value; reject longer lists rather than silently
+ dropping breakdowns.
+ """
+ if isinstance(data, dict):
+ for key in ("groupby", "breakdown"):
+ value = data.get(key)
+ if isinstance(value, list):
+ if len(value) > 1:
+ raise ValueError(
+ "waterfall breakdown is single-select; pass at "
+ "most one column"
+ )
+ data[key] = value[0] if value else None
Review Comment:
This unwrap only works when the list item is a dict. Native Superset
form_data for a plain (non-adhoc) column is typically a list of bare strings —
e.g. `groupby: ["region"]` (see `test_preview_utils.py`'s `groupby=["year"]`) —
and `value[0]` there is just `"region"`, which then fails `ColumnRef`
validation since `ColumnRef` has no before-validator accepting a string. So the
exact case this validator says it's for ("a config round-tripped from existing
waterfall form_data") breaks for the more common shape; only the `[{"name":
...}]` shape is actually tested/working.
Worth normalizing a string item into `{"name": value[0]}` (keeping dict
items as-is) before assigning.
##########
superset/mcp_service/chart/chart_utils.py:
##########
@@ -951,6 +952,37 @@ def map_box_plot_config(config: "BoxPlotChartConfig") ->
Dict[str, Any]:
return form_data
+def map_waterfall_config(config: WaterfallChartConfig) -> Dict[str, Any]:
+ """Map waterfall config to Superset form_data (viz_type waterfall).
+
+ Matches the frontend Waterfall buildQuery contract: a single ``x_axis``
+ column, an optional single-select ``groupby`` breakdown, and one
+ ``metric``; the query orders by the axis columns ascending, which the
+ frontend derives from these keys.
+ """
+ form_data: Dict[str, Any] = {
+ "viz_type": "waterfall",
+ "x_axis": config.x_axis.name,
+ "groupby": [config.breakdown.name] if config.breakdown else [],
+ "metric": create_metric_object(config.metric),
+ "show_total": config.show_total,
+ "show_legend": config.show_legend,
+ "increase_label": config.increase_label,
+ "decrease_label": config.decrease_label,
+ "total_label": config.total_label,
+ "x_axis_time_format": config.x_axis_time_format,
+ "y_axis_format": config.y_axis_format,
+ "row_limit": config.row_limit,
+ }
+ # Bucket a temporal x_axis; Superset ignores time_grain_sqla for
+ # non-temporal columns, matching the frontend control's behavior.
+ if config.time_grain:
+ form_data["time_grain_sqla"] = config.time_grain
Review Comment:
This only sets `time_grain_sqla`, not `granularity_sqla`. Elsewhere in this
file the two are always paired for a temporal column —
`configure_temporal_handling()` (used by XY) sets `granularity_sqla = x_axis`
alongside `time_grain_sqla`, and BigNumber's trendline path does the same.
Without `granularity_sqla` bound to a column here, the requested bucketing may
not actually apply.
Might be simplest to just call `configure_temporal_handling()` instead of
this bespoke block — it already handles both the pairing and the
non-temporal-column warning that's currently missing here too.
--
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]
