gkneighb commented on code in PR #41860:
URL: https://github.com/apache/superset/pull/41860#discussion_r3575621619
##########
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
+ )
+ 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(100, description="Max boxes", ge=1, le=10000)
Review Comment:
Confirmed and fixed in 3f5f5123ad — row_limit now defaults to 10000
(le=50000), matching the frontend's shared control.
##########
tests/unit_tests/mcp_service/chart/test_histogram_boxplot_charts.py:
##########
@@ -0,0 +1,371 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""Tests for the histogram and box plot chart type plugins.
+
+Schema validation, form_data mapping (matching the frontend buildQuery
+contracts for viz_type ``histogram_v2`` and ``box_plot``), and registry
+integration.
+"""
+
+import pytest
+from pydantic import TypeAdapter, ValidationError
+
+from superset.mcp_service.chart.chart_utils import (
+ map_box_plot_config,
+ map_histogram_config,
+)
+from superset.mcp_service.chart.schemas import (
+ BoxPlotChartConfig,
+ ChartConfig,
+ HistogramChartConfig,
+)
+from superset.mcp_service.common.error_schemas import ChartGenerationError
Review Comment:
Declining as inaccurate: ruff parses string annotations in typing contexts,
so the import is recognized as used — `ruff check` passes on this file locally
and in the repo's pre-commit config (no F401). CI's pre-commit run on the
current HEAD confirms.
--
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]
