gkneighb commented on code in PR #41860:
URL: https://github.com/apache/superset/pull/41860#discussion_r3575684758
##########
superset/mcp_service/chart/chart_utils.py:
##########
@@ -891,6 +893,64 @@ def map_pie_config(config: PieChartConfig) -> Dict[str,
Any]:
return form_data
+def map_histogram_config(config: "HistogramChartConfig") -> Dict[str, Any]:
+ """Map histogram config to Superset form_data (viz_type histogram_v2).
+
+ Matches the frontend Histogram buildQuery contract: a single ``column``
+ string to bin, ``groupby`` name list for series, plus bins/normalize/
+ cumulative passed straight through to the histogram post-processing
+ operator.
+ """
+ form_data: Dict[str, Any] = {
+ "viz_type": "histogram_v2",
+ "column": config.column.name,
+ "groupby": [g.name for g in (config.groupby or [])],
+ "bins": config.bins,
+ "normalize": config.normalize,
+ "cumulative": config.cumulative,
+ "row_limit": config.row_limit,
+ }
+ _add_adhoc_filters(form_data, config.filters)
+ return form_data
+
+
+# The exact strings the frontend boxplotOperator understands; the percentile
+# variant must match its PERCENTILE_REGEX: "<low>/<high> percentiles".
+_WHISKER_TYPE_TO_OPTION = {
+ "tukey": "Tukey",
+ "min_max": "Min/max (no outliers)",
+}
+
+
+def map_box_plot_config(config: "BoxPlotChartConfig") -> Dict[str, Any]:
+ """Map box plot config to Superset form_data (viz_type box_plot).
+
+ Matches the frontend BoxPlot buildQuery contract: ``columns`` are the
+ distribute-across values (one box per value), ``groupby`` the series
+ dimensions, and ``whiskerOptions`` one of the strings the
+ boxplotOperator post-processor parses.
+ """
+ if config.whisker_type == "percentile":
+ whisker_options = (
+ f"{config.percentile_low}/{config.percentile_high} percentiles"
+ )
+ else:
+ whisker_options = _WHISKER_TYPE_TO_OPTION[config.whisker_type]
+
+ form_data: Dict[str, Any] = {
+ "viz_type": "box_plot",
+ "columns": [c.name for c in config.distribute_across],
+ "groupby": [d.name for d in (config.dimensions or [])],
+ "metrics": [create_metric_object(m) for m in config.metrics],
Review Comment:
Confirmed against the pandas boxplot post-processor (its docstring: groupby
= 'the categories to group by (x-axis)') and fixed in the latest push. The
form_data key mapping was correct but the schema's semantic contract was
inverted: descriptions/examples/docstrings/name-generation now state that
dimensions (form_data groupby) splits boxes while distribute_across (form_data
columns) is the sample axis; the schema example includes dimensions so the
minimal path no longer produces a collapsed single box. Regression test asserts
the semantics explicitly.
##########
superset/mcp_service/chart/schemas.py:
##########
@@ -1846,6 +1846,123 @@ def validate_unique_column_labels(self) ->
"XYChartConfig":
return self
+class HistogramChartConfig(UnknownFieldCheckMixin):
+ """Config for histogram charts (viz_type ``histogram_v2``)."""
+
+ model_config = ConfigDict(extra="ignore", populate_by_name=True)
+
+ chart_type: Literal["histogram"] = "histogram"
+ column: ColumnRef = Field(
+ ...,
+ description="Numeric column to bin (a physical dataset column)",
+ )
+ groupby: List[ColumnRef] | None = Field(
+ None,
+ description="Optional dimensions to split the distribution into
series",
+ )
+ bins: int = Field(5, description="Number of histogram bins", ge=1, le=1000)
+ normalize: bool = Field(False, description="Normalize bin counts to
proportions")
+ cumulative: bool = Field(False, description="Accumulate bin counts left to
right")
+ 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 rows sampled", ge=1,
le=100000)
+
+ @model_validator(mode="after")
+ def reject_metric_style_column(self) -> "HistogramChartConfig":
+ """The binned column is a physical column, not a metric."""
+ _reject_sql_expression_on_dimension(self.column, "column")
+ if self.column and self.column.saved_metric:
+ raise ValueError(
+ "column cannot use saved_metric=True; histograms bin a "
+ "physical numeric column"
+ )
+ for i, col in enumerate(self.groupby or []):
+ _reject_sql_expression_on_dimension(col, f"groupby[{i}]")
+ if col.saved_metric:
+ raise ValueError(
+ f"groupby[{i}] cannot use saved_metric=True; "
+ "saved metrics are not dimensions"
+ )
+ return self
+
+
+class BoxPlotChartConfig(UnknownFieldCheckMixin):
+ """Config for box plot charts (viz_type ``box_plot``)."""
+
+ model_config = ConfigDict(extra="ignore", populate_by_name=True)
+
+ chart_type: Literal["box_plot"] = "box_plot"
+ metrics: List[ColumnRef] = Field(
+ ...,
+ min_length=1,
+ description="Metrics whose distributions are plotted (use aggregate "
+ "e.g. AVG, SUM for ad-hoc, or saved_metric=True for saved metrics)",
+ )
+ distribute_across: List[ColumnRef] = Field(
+ ...,
+ min_length=1,
+ description="Columns whose values form the boxes along the x-axis "
+ "(one box per value)",
+ )
+ dimensions: List[ColumnRef] | None = Field(
+ None,
+ description="Optional series dimensions (one colored box group per
value)",
+ )
Review Comment:
Fixed in the latest push — 'columns' and 'groupby' are accepted as
validation aliases for distribute_across/dimensions, per the codebase's
translate-don't-refuse convention, with a test.
##########
superset/mcp_service/chart/schemas.py:
##########
@@ -1846,6 +1846,123 @@ def validate_unique_column_labels(self) ->
"XYChartConfig":
return self
+class HistogramChartConfig(UnknownFieldCheckMixin):
+ """Config for histogram charts (viz_type ``histogram_v2``)."""
+
+ model_config = ConfigDict(extra="ignore", populate_by_name=True)
+
+ chart_type: Literal["histogram"] = "histogram"
+ column: ColumnRef = Field(
+ ...,
+ description="Numeric column to bin (a physical dataset column)",
+ )
+ groupby: List[ColumnRef] | None = Field(
+ None,
+ description="Optional dimensions to split the distribution into
series",
+ )
+ bins: int = Field(5, description="Number of histogram bins", ge=1, le=1000)
+ normalize: bool = Field(False, description="Normalize bin counts to
proportions")
+ cumulative: bool = Field(False, description="Accumulate bin counts left to
right")
+ 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 rows sampled", ge=1,
le=100000)
+
+ @model_validator(mode="after")
+ def reject_metric_style_column(self) -> "HistogramChartConfig":
+ """The binned column is a physical column, not a metric."""
+ _reject_sql_expression_on_dimension(self.column, "column")
+ if self.column and self.column.saved_metric:
+ raise ValueError(
+ "column cannot use saved_metric=True; histograms bin a "
+ "physical numeric column"
+ )
+ for i, col in enumerate(self.groupby or []):
+ _reject_sql_expression_on_dimension(col, f"groupby[{i}]")
+ if col.saved_metric:
+ raise ValueError(
+ f"groupby[{i}] cannot use saved_metric=True; "
+ "saved metrics are not dimensions"
+ )
+ return self
+
+
+class BoxPlotChartConfig(UnknownFieldCheckMixin):
+ """Config for box plot charts (viz_type ``box_plot``)."""
+
+ model_config = ConfigDict(extra="ignore", populate_by_name=True)
+
+ chart_type: Literal["box_plot"] = "box_plot"
+ metrics: List[ColumnRef] = Field(
+ ...,
+ min_length=1,
+ description="Metrics whose distributions are plotted (use aggregate "
+ "e.g. AVG, SUM for ad-hoc, or saved_metric=True for saved metrics)",
+ )
+ distribute_across: List[ColumnRef] = Field(
+ ...,
+ min_length=1,
+ description="Columns whose values form the boxes along the x-axis "
+ "(one box per value)",
+ )
+ dimensions: List[ColumnRef] | None = Field(
+ None,
+ description="Optional series dimensions (one colored box group per
value)",
+ )
+ whisker_type: Literal["tukey", "min_max", "percentile"] = Field(
+ "tukey",
+ description="Whisker algorithm: 'tukey' (1.5 IQR), 'min_max' (no "
+ "outliers), or 'percentile' (requires percentile_low/percentile_high)",
+ )
+ percentile_low: int | None = Field(
+ None, description="Lower whisker percentile (0-100)", ge=0, le=100
+ )
+ percentile_high: int | None = Field(
+ None, description="Upper whisker percentile (0-100)", ge=0, le=100
+ )
Review Comment:
Fixed in the latest push — whiskerOptions display strings (Tukey, Min/max
(no outliers), '<low>/<high> percentiles') are translated by a before-validator
into whisker_type/percentiles; unsupported strings get a clear error pointing
at whisker_type. Tests cover translation and rejection.
--
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]
