diff --git a/packages/markitdown-ocr/src/markitdown_ocr/_pptx_converter_with_ocr.py b/packages/markitdown-ocr/src/markitdown_ocr/_pptx_converter_with_ocr.py index 7e91ed6b4..8f0d4a3d5 100644 --- a/packages/markitdown-ocr/src/markitdown_ocr/_pptx_converter_with_ocr.py +++ b/packages/markitdown-ocr/src/markitdown_ocr/_pptx_converter_with_ocr.py @@ -242,8 +242,7 @@ def _convert_chart_to_markdown(self, chart): header = markdown_table[0] separator = "|" + "|".join(["---"] * len(data[0])) + "|" return md + "\\n".join([header, separator] + markdown_table[1:]) - except ValueError as e: - if "unsupported plot type" in str(e): - return "\\n\\n[unsupported chart]\\n\\n" except Exception: + # Every ValueError other than "unsupported plot type" used to fall + # through and return None, which the caller then concatenated. return "\\n\\n[unsupported chart]\\n\\n" diff --git a/packages/markitdown/src/markitdown/converters/_pptx_converter.py b/packages/markitdown/src/markitdown/converters/_pptx_converter.py index 21a12dc81..da62a06bd 100644 --- a/packages/markitdown/src/markitdown/converters/_pptx_converter.py +++ b/packages/markitdown/src/markitdown/converters/_pptx_converter.py @@ -328,10 +328,11 @@ def _convert_chart_to_markdown(self, chart): header = markdown_table[0] separator = "|" + "|".join(["---"] * len(data[0])) + "|" return md + "\n".join([header, separator] + markdown_table[1:]) - except ValueError as e: - # Handle the specific error for unsupported chart types - if "unsupported plot type" in str(e): - return "\n\n[unsupported chart]\n\n" except Exception: - # Catch any other exceptions that might occur + # Any failure to read the chart degrades to a placeholder. This + # includes the ValueError("unsupported plot type ...") that + # python-pptx raises for chart types it cannot model, which used to + # be the only ValueError handled here: every other one fell through + # and returned None, which the caller then concatenated onto + # md_content, failing the whole presentation with a TypeError. return "\n\n[unsupported chart]\n\n" diff --git a/packages/markitdown/tests/test_pptx_chart_errors.py b/packages/markitdown/tests/test_pptx_chart_errors.py new file mode 100644 index 000000000..ad68f2114 --- /dev/null +++ b/packages/markitdown/tests/test_pptx_chart_errors.py @@ -0,0 +1,113 @@ +#!/usr/bin/env python3 -m pytest +"""Tests for chart reads that fail inside PptxConverter. + +``_convert_chart_to_markdown`` wraps its body in two handlers that both return +the same ``[unsupported chart]`` placeholder, but the ``ValueError`` handler +returned it only when the message contained "unsupported plot type". Any other +``ValueError`` reached the end of the handler and fell off the function, so the +method returned ``None``. + +The caller does ``md_content += self._convert_chart_to_markdown(shape.chart)`` +unguarded, so that ``None`` raises ``TypeError: can only concatenate str (not +"NoneType") to str``. ``MarkItDown._convert`` catches it and, with no other +converter accepting a .pptx, re-raises as ``FileConversionException`` -- one +unreadable chart therefore loses the entire presentation rather than a single +shape. +""" + +import io +from unittest.mock import patch + +import pytest +from pptx import Presentation +from pptx.chart.chart import Chart +from pptx.chart.data import CategoryChartData +from pptx.enum.chart import XL_CHART_TYPE +from pptx.util import Inches + +from markitdown import MarkItDown +from markitdown._stream_info import StreamInfo +from markitdown.converters._pptx_converter import PptxConverter + +PLACEHOLDER = "[unsupported chart]" + +# A ValueError python-pptx can raise that is not the special-cased one. +OTHER_VALUE_ERROR = "chart has no categories" + + +def _deck_with_chart() -> bytes: + """A minimal one-slide deck holding a plain clustered column chart.""" + presentation = Presentation() + slide = presentation.slides.add_slide(presentation.slide_layouts[5]) + chart_data = CategoryChartData() + chart_data.categories = ["A", "B"] + chart_data.add_series("S1", (1.0, 2.0)) + slide.shapes.add_chart( + XL_CHART_TYPE.COLUMN_CLUSTERED, + Inches(1), + Inches(1), + Inches(4), + Inches(3), + chart_data, + ) + buffer = io.BytesIO() + presentation.save(buffer) + return buffer.getvalue() + + +def _convert(deck: bytes) -> str: + return ( + MarkItDown() + .convert_stream(io.BytesIO(deck), stream_info=StreamInfo(extension=".pptx")) + .markdown + ) + + +class _ChartRaising: + """Stands in for a chart whose first access raises.""" + + def __init__(self, message: str): + self._message = message + + @property + def has_title(self): + raise ValueError(self._message) + + +@pytest.mark.parametrize("message", [OTHER_VALUE_ERROR, "unsupported plot type xlFoo"]) +def test_chart_read_failure_returns_a_string(message: str) -> None: + """The method must never hand the caller a None to concatenate.""" + result = PptxConverter()._convert_chart_to_markdown(_ChartRaising(message)) + + assert isinstance(result, str) + assert PLACEHOLDER in result + + +def test_deck_survives_an_unreadable_chart() -> None: + """A chart that cannot be read costs the chart, not the presentation.""" + deck = _deck_with_chart() + + def _raise(self): + raise ValueError(OTHER_VALUE_ERROR) + + with patch.object(Chart, "plots", property(_raise)): + markdown = _convert(deck) + + assert PLACEHOLDER in markdown + + +def test_readable_chart_is_still_converted() -> None: + """Control: an ordinary chart must keep converting to a table.""" + markdown = _convert(_deck_with_chart()) + + assert "### Chart" in markdown + assert PLACEHOLDER not in markdown + assert "| Category | S1 |" in markdown + + +if __name__ == "__main__": + test_chart_read_failure_returns_a_string(OTHER_VALUE_ERROR) + test_chart_read_failure_returns_a_string("unsupported plot type xlFoo") + test_deck_survives_an_unreadable_chart() + test_readable_chart_is_still_converted() + print("All tests passed!")