gkneighb commented on code in PR #42070:
URL: https://github.com/apache/superset/pull/42070#discussion_r3605460869
##########
superset/mcp_service/chart/schemas.py:
##########
@@ -2009,6 +2009,112 @@ 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. Native form_data also names physical columns as
+ bare strings, so a string is coerced to ``{"name": ...}``.
+ """
+
+ def _coerce(value: Any) -> Any:
+ if isinstance(value, list):
+ if len(value) > 1:
+ raise ValueError(
+ "waterfall breakdown is single-select; pass at most
one column"
+ )
+ value = value[0] if value else None
+ if isinstance(value, str):
+ return {"name": value}
+ return value
+
+ if isinstance(data, dict):
+ for key in ("groupby", "breakdown"):
Review Comment:
Addressed in 10daf20775 (pushed just after this review) — the
before-validator now also coerces a bare-string x_axis to {'name': ...}, so
native form_data round-trips. Test: test_x_axis_string_coerced.
##########
superset/mcp_service/chart/chart_utils.py:
##########
@@ -951,6 +952,41 @@ 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: the grain (time_grain_sqla) needs the temporal
+ # column it applies to (granularity_sqla), mirroring the xy path's
+ # configure_temporal_handling and the frontend buildQuery's
+ # `x_axis || granularity_sqla`. Providing time_grain signals temporal
Review Comment:
Addressed in 10daf20775 — since map_waterfall_config has no dataset context,
the temporal check lives in the plugin's post_map_validate (which does receive
dataset_id): time_grain on a non-temporal x_axis returns a
non_temporal_waterfall_grain error via is_column_truly_temporal, mirroring the
histogram numeric-column check. Tests cover both directions.
--
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]
