gkneighb commented on code in PR #43570:
URL: https://github.com/apache/superset/pull/43570#discussion_r3972152675
##########
superset/mcp_service/chart/chart_utils.py:
##########
@@ -1067,6 +1068,26 @@ def map_gauge_config(config: GaugeChartConfig) ->
Dict[str, Any]:
return form_data
+def map_heatmap_config(config: HeatmapChartConfig) -> Dict[str, Any]:
+ """Map heatmap config to Superset form_data (viz_type ``heatmap_v2``).
+
+ Matches the frontend Heatmap buildQuery contract: an ``x_axis`` column and
+ a single ``groupby`` Y column form the two axes, one ``metric`` colours
+ the cells, and ``normalize_across`` selects the rank-normalization range.
+ The Y axis is a single-select ``groupby`` (not a list).
+ """
+ form_data: Dict[str, Any] = {
+ "viz_type": "heatmap_v2",
+ "x_axis": config.x_axis.name,
+ "groupby": config.y_axis.name,
Review Comment:
Reproduced exactly — `columns_from_form_data` raised `AttributeError: 'str'
object has no attribute 'copy'` on the scalar `groupby`, and neither
`_compile_chart` nor the tool's outer handler caught it. Fixed in the shared
helper as you suggested rather than in the mapper, since the scalar is the
faithful shape (`groupby` is `multi: false`, and `MigrateHeatmapChart` renames
`all_columns_y` straight to the scalar).
`columns_from_form_data` now runs a `_as_column_list` coercion on `groupby`,
`columns`, and the raw-mode branch, wrapping a scalar in a one-element list,
mirroring `chart_helpers.resolve_groupby`. That also removes the latent crash
on the dashboard Excel export path for migrated heatmap charts, which reaches
the same helper via `_columns_and_metrics`.
Tests: `tests/unit_tests/common/test_form_data_query_context.py` gets
`test_columns_scalar_groupby_is_coerced_to_list` (`{"x_axis": "day", "groupby":
"hour"}` → `["day", "hour"]`) and a scalar-`columns` sibling; the heatmap suite
gets `test_x_axis_reaches_columns_from_form_data`, which goes through
`columns_from_form_data` rather than `chart_helpers` so the two builders are
both guarded.
##########
superset/mcp_service/chart/schemas.py:
##########
@@ -1121,6 +1121,61 @@ def reject_inverted_bounds(self) -> "GaugeChartConfig":
return self
+class HeatmapChartConfig(BaseChartConfig):
+ """Config for heatmap charts (viz_type ``heatmap_v2``).
+
+ Matches the frontend Heatmap buildQuery contract: an ``x_axis`` column, a
+ single ``groupby`` column for the Y axis, and one ``metric`` colouring each
+ cell. ``normalize_across`` drives the server-side rank normalization
+ (whole heatmap, per-x, or per-y).
+ """
+
+ model_config = ConfigDict(extra="ignore", populate_by_name=True)
+
+ chart_type: Literal["heatmap_v2"] = "heatmap_v2"
+ x_axis: ColumnRef = Field(
+ ...,
+ description="Column along the X axis",
+ )
+ y_axis: ColumnRef = Field(
+ ...,
+ description="Column along the Y axis (form_data 'groupby';
single-select)",
+ validation_alias=AliasChoices("y_axis", "groupby"),
+ )
+ metric: ColumnRef = Field(
+ ...,
+ description="Value metric colouring each cell (use aggregate e.g. SUM,
"
+ "COUNT for ad-hoc, or set saved_metric=True for a saved dataset
metric)",
+ )
+ normalize_across: Literal["heatmap", "x", "y"] = Field(
Review Comment:
Fixed. `HeatmapChartConfig` now exposes `normalized: bool = False`, and the
mapper threads it into `form_data`, so `normalize_across` is no longer inert on
the frontend path — with `normalized=true` the rank column becomes
`colorColumn`. Both field descriptions now state that `normalize_across` only
takes effect when `normalized=true`. Mapping tests assert the default (`False`)
and the pass-through (`True`).
The server-side `rankOperator` post-processing remains the tracked
follow-up, as agreed.
##########
superset/mcp_service/chart/schemas.py:
##########
@@ -1121,6 +1121,61 @@ def reject_inverted_bounds(self) -> "GaugeChartConfig":
return self
+class HeatmapChartConfig(BaseChartConfig):
Review Comment:
Intentional, deferred. This PR keeps the field set minimal (the stated
scope), and `time_grain` only bites with a temporal x_axis. It is a fair
query-contract gap, so I will add it in the same follow-up as the
`normalize_across` server-side work, matching the `granularity_sqla` mirroring
waterfall already does — unless you would prefer it folded in here.
--
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]
