Skip to content
22 changes: 18 additions & 4 deletions superset/mcp_service/chart/ascii_charts.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,11 @@ def _generate_ascii_bar_chart(data: list[Any], width: int, height: int) -> str:
label_val = None

for _key, val in row.items():
if isinstance(val, (int, float)) and numeric_val is None:
if (
isinstance(val, (int, float))
and not _is_nan_value(val)
and numeric_val is None
):
Comment thread
aminghadersohi marked this conversation as resolved.
numeric_val = val
elif isinstance(val, str) and label_val is None:
label_val = val
Expand Down Expand Up @@ -121,7 +125,10 @@ def _generate_horizontal_bar_chart(
# Calculate bar length
if max_val > min_val:
normalized = (value - min_val) / (max_val - min_val)
bar_length = max(1, int(normalized * max_bar_width))
if _is_nan_value(normalized):
bar_length = 0
else:
bar_length = max(1, int(normalized * max_bar_width))
else:
bar_length = 1

Expand Down Expand Up @@ -164,7 +171,10 @@ def _generate_vertical_bar_chart( # noqa: C901
for col, value in enumerate(values):
if max_val > min_val:
normalized = (value - min_val) / (max_val - min_val)
bar_height = max(1, int(normalized * chart_height))
if _is_nan_value(normalized):
bar_height = 0
else:
bar_height = max(1, int(normalized * chart_height))
else:
bar_height = 1

Expand Down Expand Up @@ -274,7 +284,11 @@ def _extract_time_series_data(data: list[Any]) -> tuple[list[float], list[str]]:
label_val = None

for key, val in row.items():
if isinstance(val, (int, float)) and numeric_val is None:
if (
isinstance(val, (int, float))
and not _is_nan_value(val)
and numeric_val is None
):
Comment thread
aminghadersohi marked this conversation as resolved.
numeric_val = val
elif isinstance(val, str) and label_val is None:
# Use the key name if it looks like a date/time field
Expand Down
106 changes: 106 additions & 0 deletions tests/unit_tests/mcp_service/chart/test_ascii_charts.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
# 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.

"""Unit tests for ascii_charts.py NaN/null value handling."""

from superset.mcp_service.chart.ascii_charts import generate_ascii_chart


def test_bar_chart_with_null_values_does_not_raise() -> None:
"""Bar chart renderer must not crash when dataset rows contain NaN values."""
data = [
{"category": "A", "value": 10.0},
{"category": "B", "value": float("nan")},
{"category": "C", "value": 30.0},
]
result = generate_ascii_chart(data, "bar")
assert isinstance(result, str)
assert len(result) > 0


def test_bar_chart_with_all_null_values_returns_fallback() -> None:
"""Bar chart with no valid numeric rows should return a fallback message."""
data = [
{"category": "A", "value": float("nan")},
{"category": "B", "value": float("nan")},
]
result = generate_ascii_chart(data, "bar")
assert isinstance(result, str)
assert len(result) > 0
Comment thread
aminghadersohi marked this conversation as resolved.
Outdated


def test_line_chart_with_null_values_does_not_raise() -> None:
"""Line chart renderer must not crash when dataset rows contain NaN values."""
data = [
{"date": "2024-01", "sales": 100.0},
{"date": "2024-02", "sales": float("nan")},
{"date": "2024-03", "sales": 300.0},
]
result = generate_ascii_chart(data, "line")
assert isinstance(result, str)
assert len(result) > 0


def test_horizontal_bar_chart_nan_rows_are_skipped() -> None:
"""NaN rows must be silently skipped; valid rows render normally."""
# Use labels longer than 8 chars to force horizontal layout, where full
# label text is preserved (vertical layout truncates to 3 chars).
data = [
{"label": "Alpha Category", "amount": 50.0},
{"label": "Beta Category", "amount": float("nan")},
{"label": "Gamma Category", "amount": 150.0},
]
result = generate_ascii_chart(data, "bar")
Comment thread
aminghadersohi marked this conversation as resolved.
# Valid labels should appear in output; NaN row should not crash
assert "Alpha" in result or "Gamma" in result


def test_column_chart_with_null_values_does_not_raise() -> None:
"""Column (vertical bar) chart must not crash on NaN values."""
data = [
{"x": "Q1", "y": 10.0},
{"x": "Q2", "y": float("nan")},
{"x": "Q3", "y": 30.0},
{"x": "Q4", "y": 40.0},
]
result = generate_ascii_chart(data, "column")
assert isinstance(result, str)
assert len(result) > 0


def test_timeseries_bar_with_null_values_does_not_raise() -> None:
"""echarts_timeseries_bar chart type must not crash on NaN values."""
data = [
{"ts": "2024-01-01", "count": 5.0},
{"ts": "2024-01-02", "count": float("nan")},
{"ts": "2024-01-03", "count": 15.0},
]
result = generate_ascii_chart(data, "echarts_timeseries_bar")
assert isinstance(result, str)
assert len(result) > 0


def test_chart_with_none_values_does_not_raise() -> None:
"""None (SQL NULL) values should be treated identically to NaN."""
data = [
{"name": "X", "metric": 100.0},
{"name": "Y", "metric": None},
{"name": "Z", "metric": 200.0},
]
result = generate_ascii_chart(data, "bar")
assert isinstance(result, str)
assert len(result) > 0
Loading