From 741a3cff1da25745e521b603ca1eef87f14f98ff Mon Sep 17 00:00:00 2001 From: aslamalkarywk7 Date: Fri, 18 Sep 2026 23:51:03 +0000 Subject: [PATCH 1/8] fix(xlsx): keep currency labels from number formats (microsoft/markitdown#53) --- .../markitdown/converters/_xlsx_converter.py | 110 ++++++++++++++++++ .../markitdown/tests/test_xlsx_currency.py | 70 +++++++++++ 2 files changed, 180 insertions(+) create mode 100644 packages/markitdown/tests/test_xlsx_currency.py diff --git a/packages/markitdown/src/markitdown/converters/_xlsx_converter.py b/packages/markitdown/src/markitdown/converters/_xlsx_converter.py index 9f794a3b7..cc79a2b2a 100644 --- a/packages/markitdown/src/markitdown/converters/_xlsx_converter.py +++ b/packages/markitdown/src/markitdown/converters/_xlsx_converter.py @@ -37,6 +37,113 @@ ACCEPTED_XLS_FILE_EXTENSIONS = [".xls"] +# Currency symbols that may appear in Excel number formats, either as quoted +# literals (e.g. '"$"#,##0.00') or locale blocks (e.g. '[$€-x-euro2]'). +# See https://github.com/microsoft/markitdown/issues/53. +_CURRENCY_SYMBOL_RE = re.compile(r'[$€£¥₹₽₩₪₺₴₸₫₦¤]') +_LOCALE_BLOCK_RE = re.compile(r'\[\$([^\]-]+)') +_QUOTED_LITERAL_RE = re.compile(r'"([^"]*)"') + + +def _currency_symbol(number_format: Any) -> Optional[str]: + """Return the currency symbol of an Excel number format, if it has one. + + Only the first (`;`-separated) section is considered, matching how + positive values are rendered. + """ + if not isinstance(number_format, str): + return None + first_section = number_format.split(';')[0] + quoted = ''.join(_QUOTED_LITERAL_RE.findall(first_section)) + locale = ''.join(_LOCALE_BLOCK_RE.findall(first_section)) + candidates = quoted + locale + if not candidates: + candidates = first_section + match = _CURRENCY_SYMBOL_RE.search(candidates) + return match.group(0) if match else None + + +def _is_currency_position_prefix(number_format: str) -> bool: + """Decide whether the currency symbol renders before the value.""" + first_section = number_format.split(';')[0] + symbol_match = _CURRENCY_SYMBOL_RE.search(first_section) + placeholder_match = re.search(r'[#0?]', first_section) + if not symbol_match or not placeholder_match: + return True + return symbol_match.start() < placeholder_match.start() + + +def _overlay_currency_labels( + sheets: dict[str, Any], workbook_stream: BinaryIO +) -> None: + """Rewrite currency-formatted numeric cells with their display label. + + `pandas.read_excel` returns raw values and drops Excel number formats, so + currency-formatted cells lose their label (e.g. 1199 instead of $1199). + This overlays the label in place so the downstream HTML/markdown table + shows what the spreadsheet shows. DataFrames are mutated in place. + """ + import openpyxl # Local import: already a required dependency (see above). + + position = workbook_stream.tell() + try: + workbook_stream.seek(0) + workbook = openpyxl.load_workbook( + workbook_stream, data_only=True, read_only=True + ) + except Exception: + return + try: + for worksheet in workbook.worksheets: + frame = sheets.get(worksheet.title) + if frame is None: + continue + object_cols: set[int] = set() + # pandas treats the first row as the header, so data row i lives in + # openpyxl row i + 2 (both 1-indexed vs 0-indexed and header offset). + for openpyxl_row in worksheet.iter_rows(min_row=2): + for cell in openpyxl_row: + value = cell.value + if ( + value is None + or isinstance(value, bool) + or not isinstance(value, (int, float)) + ): + continue + symbol = _currency_symbol(cell.number_format) + if symbol is None: + continue + data_row = cell.row - 2 + data_col = cell.column - 1 + if not (0 <= data_row < len(frame)) or not ( + 0 <= data_col < len(frame.columns) + ): + continue + text = str(frame.iat[data_row, data_col]) + if symbol in text: + continue + if data_col not in object_cols: + # A str label cannot live in a numeric column: widen it once. + frame[frame.columns[data_col]] = frame[ + frame.columns[data_col] + ].astype(object) + object_cols.add(data_col) + if _is_currency_position_prefix(str(cell.number_format)): + text = f'{symbol}{text}' + else: + text = f'{text}{symbol}' + frame.iat[data_row, data_col] = text + finally: + try: + workbook.close() + except Exception: + pass + try: + workbook_stream.seek(position) + except Exception: + pass + + # Some producers write the legacy attribute "showZeroes" on , where the # schema calls it "showZeros". openpyxl rejects the unknown attribute outright, so the # workbook is repaired by renaming it. The rename is confined to start tags. @@ -142,6 +249,9 @@ def convert( md_content = "" with _read_xlsx_sheets(file_stream) as (sheets, workbook_stream): + # pandas drops Excel number formats: overlay currency labels so + # currency-formatted cells render as the spreadsheet shows them. + _overlay_currency_labels(sheets, workbook_stream) images = None if type(self)._image_to_html is not XlsxConverter._image_to_html: from ..converter_utils._xlsx_images import _XlsxImages diff --git a/packages/markitdown/tests/test_xlsx_currency.py b/packages/markitdown/tests/test_xlsx_currency.py new file mode 100644 index 000000000..fe8599bcf --- /dev/null +++ b/packages/markitdown/tests/test_xlsx_currency.py @@ -0,0 +1,70 @@ +"""Currency-formatted Excel cells keep their label (microsoft/markitdown#53).""" + +import io + +import openpyxl +import pytest + +from markitdown import StreamInfo +from markitdown.converters import XlsxConverter + + +_INFO = StreamInfo(extension=".xlsx") + + +def _workbook() -> bytes: + workbook = openpyxl.Workbook() + sheet = workbook.active + sheet.title = "Sheet1" + sheet.append(["Item", "Count", "Cost", "Weight", "Total"]) + sheet.append(["Breakfast", 20, 5, None, 100]) + sheet.append(["Laptops", 5, 1199, None, 5995]) + sheet.append(["Car tires", 8, 199, "150 kg", 1592]) + for row in sheet.iter_rows(min_row=2, min_col=3, max_col=3): + for cell in row: + cell.number_format = '"$"#,##0.00' + for row in sheet.iter_rows(min_row=2, min_col=5, max_col=5): + for cell in row: + cell.number_format = '"$"#,##0.00' + stream = io.BytesIO() + workbook.save(stream) + workbook.close() + return stream.getvalue() + + +def _convert(data: bytes) -> str: + return XlsxConverter().convert(io.BytesIO(data), _INFO).markdown + + +def test_currency_cells_keep_their_label() -> None: + markdown = _convert(_workbook()) + assert "$5" in markdown + assert "$1199" in markdown + assert "$100" in markdown + assert "Breakfast" in markdown + assert "150 kg" in markdown + + +def test_plain_cells_are_untouched() -> None: + workbook = openpyxl.Workbook() + sheet = workbook.active + sheet.append(["Item", "Count"]) + sheet.append(["Breakfast", 20]) + stream = io.BytesIO() + workbook.save(stream) + workbook.close() + markdown = _convert(stream.getvalue()) + assert "| Breakfast | 20 |" in markdown + assert "$" not in markdown + + +def test_euro_suffix_format() -> None: + workbook = openpyxl.Workbook() + sheet = workbook.active + sheet.append(["Price"]) + sheet.append([42]) + sheet["A2"].number_format = '#,##0.00 [$€-x-euro2]' + stream = io.BytesIO() + workbook.save(stream) + workbook.close() + assert "42€" in _convert(stream.getvalue()) From da00881a6971a2660a5c2fd350868f83885d336b Mon Sep 17 00:00:00 2001 From: aslamalkarywk7 Date: Sun, 20 Sep 2026 11:00:22 +0000 Subject: [PATCH 2/8] fix(xlsx): select currency section by cell value Negative values use the second Excel format section (zero can use the third), so inspect the section matching the cell sign instead of always using the first section. --- .../markitdown/converters/_xlsx_converter.py | 90 +++++++++++++------ .../markitdown/tests/test_xlsx_currency.py | 38 ++++++++ 2 files changed, 101 insertions(+), 27 deletions(-) diff --git a/packages/markitdown/src/markitdown/converters/_xlsx_converter.py b/packages/markitdown/src/markitdown/converters/_xlsx_converter.py index cc79a2b2a..2a6654863 100644 --- a/packages/markitdown/src/markitdown/converters/_xlsx_converter.py +++ b/packages/markitdown/src/markitdown/converters/_xlsx_converter.py @@ -45,32 +45,68 @@ _QUOTED_LITERAL_RE = re.compile(r'"([^"]*)"') -def _currency_symbol(number_format: Any) -> Optional[str]: - """Return the currency symbol of an Excel number format, if it has one. +def _select_format_section(number_format: str, value: Any) -> str: + """Pick the `;`-separated Excel section that applies to `value`. - Only the first (`;`-separated) section is considered, matching how - positive values are rendered. - """ - if not isinstance(number_format, str): - return None - first_section = number_format.split(';')[0] - quoted = ''.join(_QUOTED_LITERAL_RE.findall(first_section)) - locale = ''.join(_LOCALE_BLOCK_RE.findall(first_section)) - candidates = quoted + locale - if not candidates: - candidates = first_section - match = _CURRENCY_SYMBOL_RE.search(candidates) - return match.group(0) if match else None - - -def _is_currency_position_prefix(number_format: str) -> bool: - """Decide whether the currency symbol renders before the value.""" - first_section = number_format.split(';')[0] - symbol_match = _CURRENCY_SYMBOL_RE.search(first_section) - placeholder_match = re.search(r'[#0?]', first_section) - if not symbol_match or not placeholder_match: - return True - return symbol_match.start() < placeholder_match.start() + Excel uses: 1 section = all numbers, 2 sections = positive+zero / negative, + 3 sections = positive / negative / zero (4th section is text and ignored). + """ + parts = number_format.split(";") + if len(parts) == 1: + return parts[0] + if len(parts) == 2: + # Zero renders with the first section when only two are present. + try: + is_negative = float(value) < 0 + except Exception: + is_negative = False + return parts[1] if is_negative else parts[0] + try: + numeric = float(value) + except Exception: + return parts[0] + if numeric > 0: + return parts[0] + if numeric < 0: + return parts[1] + return parts[2] + + +def _currency_symbol(number_format: Any, value: Any = None) -> Optional[str]: + """Return the currency symbol of an Excel number format, if it has one. + + When `value` is given, the `;`-separated section matching its sign is + inspected (positive / negative / zero). Otherwise the first section is + used, matching how positive values are rendered. + """ + if not isinstance(number_format, str): + return None + section = ( + _select_format_section(number_format, value) + if value is not None + else number_format.split(";")[0] + ) + quoted = "".join(_QUOTED_LITERAL_RE.findall(section)) + locale = "".join(_LOCALE_BLOCK_RE.findall(section)) + candidates = quoted + locale + if not candidates: + candidates = section + match = _CURRENCY_SYMBOL_RE.search(candidates) + return match.group(0) if match else None + + +def _is_currency_position_prefix(number_format: str, value: Any = None) -> bool: + """Decide whether the currency symbol renders before the value.""" + section = ( + _select_format_section(number_format, value) + if value is not None + else number_format.split(";")[0] + ) + symbol_match = _CURRENCY_SYMBOL_RE.search(section) + placeholder_match = re.search(r"[#0?]", section) + if not symbol_match or not placeholder_match: + return True + return symbol_match.start() < placeholder_match.start() def _overlay_currency_labels( @@ -110,7 +146,7 @@ def _overlay_currency_labels( or not isinstance(value, (int, float)) ): continue - symbol = _currency_symbol(cell.number_format) + symbol = _currency_symbol(cell.number_format, value) if symbol is None: continue data_row = cell.row - 2 @@ -128,7 +164,7 @@ def _overlay_currency_labels( frame.columns[data_col] ].astype(object) object_cols.add(data_col) - if _is_currency_position_prefix(str(cell.number_format)): + if _is_currency_position_prefix(str(cell.number_format), value): text = f'{symbol}{text}' else: text = f'{text}{symbol}' diff --git a/packages/markitdown/tests/test_xlsx_currency.py b/packages/markitdown/tests/test_xlsx_currency.py index fe8599bcf..b36d952d8 100644 --- a/packages/markitdown/tests/test_xlsx_currency.py +++ b/packages/markitdown/tests/test_xlsx_currency.py @@ -68,3 +68,41 @@ def test_euro_suffix_format() -> None: workbook.save(stream) workbook.close() assert "42€" in _convert(stream.getvalue()) + + +def test_section_specific_currency_uses_cell_value() -> None: + from markitdown.converters._xlsx_converter import ( + _currency_symbol, + _is_currency_position_prefix, + _select_format_section, + ) + + fmt = '"$"#,##0;"€"#,##0' + assert _select_format_section(fmt, 5) == '"$"#,##0' + assert _select_format_section(fmt, -5) == '"€"#,##0' + assert _currency_symbol(fmt, 5) == "$" + assert _currency_symbol(fmt, -5) == "€" + assert _is_currency_position_prefix(fmt, 5) + assert _is_currency_position_prefix(fmt, -5) + + # 3 sections: positive / negative / zero + fmt3 = '"$"#,##0;"€"#,##0;"¥"#,##0' + assert _currency_symbol(fmt3, 5) == "$" + assert _currency_symbol(fmt3, -5) == "€" + assert _currency_symbol(fmt3, 0) == "¥" + + # end-to-end: negative keeps its own section currency + workbook = openpyxl.Workbook() + sheet = workbook.active + sheet.append(["Balance"]) + sheet.append([10]) + sheet.append([-5]) + sheet["A2"].number_format = fmt + sheet["A3"].number_format = fmt + stream = io.BytesIO() + workbook.save(stream) + workbook.close() + markdown = _convert(stream.getvalue()) + assert "$10" in markdown + assert "€" in markdown + assert "$-5" not in markdown.replace("$-5", "") or "€" in markdown From f4a7ed1200b21f2409402f8a55fd9471c4c23cbc Mon Sep 17 00:00:00 2001 From: Adam Fourney Date: Mon, 21 Sep 2026 15:48:22 -0700 Subject: [PATCH 3/8] Fixed formatting. --- .../markitdown/converters/_xlsx_converter.py | 130 +++++++++--------- .../markitdown/tests/test_xlsx_currency.py | 2 +- 2 files changed, 65 insertions(+), 67 deletions(-) diff --git a/packages/markitdown/src/markitdown/converters/_xlsx_converter.py b/packages/markitdown/src/markitdown/converters/_xlsx_converter.py index 2a6654863..d2ebae4c8 100644 --- a/packages/markitdown/src/markitdown/converters/_xlsx_converter.py +++ b/packages/markitdown/src/markitdown/converters/_xlsx_converter.py @@ -40,8 +40,8 @@ # Currency symbols that may appear in Excel number formats, either as quoted # literals (e.g. '"$"#,##0.00') or locale blocks (e.g. '[$€-x-euro2]'). # See https://github.com/microsoft/markitdown/issues/53. -_CURRENCY_SYMBOL_RE = re.compile(r'[$€£¥₹₽₩₪₺₴₸₫₦¤]') -_LOCALE_BLOCK_RE = re.compile(r'\[\$([^\]-]+)') +_CURRENCY_SYMBOL_RE = re.compile(r"[$€£¥₹₽₩₪₺₴₸₫₦¤]") +_LOCALE_BLOCK_RE = re.compile(r"\[\$([^\]-]+)") _QUOTED_LITERAL_RE = re.compile(r'"([^"]*)"') @@ -109,75 +109,73 @@ def _is_currency_position_prefix(number_format: str, value: Any = None) -> bool: return symbol_match.start() < placeholder_match.start() -def _overlay_currency_labels( - sheets: dict[str, Any], workbook_stream: BinaryIO -) -> None: - """Rewrite currency-formatted numeric cells with their display label. +def _overlay_currency_labels(sheets: dict[str, Any], workbook_stream: BinaryIO) -> None: + """Rewrite currency-formatted numeric cells with their display label. - `pandas.read_excel` returns raw values and drops Excel number formats, so - currency-formatted cells lose their label (e.g. 1199 instead of $1199). - This overlays the label in place so the downstream HTML/markdown table - shows what the spreadsheet shows. DataFrames are mutated in place. - """ - import openpyxl # Local import: already a required dependency (see above). + `pandas.read_excel` returns raw values and drops Excel number formats, so + currency-formatted cells lose their label (e.g. 1199 instead of $1199). + This overlays the label in place so the downstream HTML/markdown table + shows what the spreadsheet shows. DataFrames are mutated in place. + """ + import openpyxl # Local import: already a required dependency (see above). - position = workbook_stream.tell() - try: - workbook_stream.seek(0) - workbook = openpyxl.load_workbook( - workbook_stream, data_only=True, read_only=True - ) - except Exception: - return - try: - for worksheet in workbook.worksheets: - frame = sheets.get(worksheet.title) - if frame is None: - continue - object_cols: set[int] = set() - # pandas treats the first row as the header, so data row i lives in - # openpyxl row i + 2 (both 1-indexed vs 0-indexed and header offset). - for openpyxl_row in worksheet.iter_rows(min_row=2): - for cell in openpyxl_row: - value = cell.value - if ( - value is None - or isinstance(value, bool) - or not isinstance(value, (int, float)) - ): - continue - symbol = _currency_symbol(cell.number_format, value) - if symbol is None: - continue - data_row = cell.row - 2 - data_col = cell.column - 1 - if not (0 <= data_row < len(frame)) or not ( - 0 <= data_col < len(frame.columns) - ): - continue - text = str(frame.iat[data_row, data_col]) - if symbol in text: - continue - if data_col not in object_cols: - # A str label cannot live in a numeric column: widen it once. - frame[frame.columns[data_col]] = frame[ - frame.columns[data_col] - ].astype(object) - object_cols.add(data_col) - if _is_currency_position_prefix(str(cell.number_format), value): - text = f'{symbol}{text}' - else: - text = f'{text}{symbol}' - frame.iat[data_row, data_col] = text - finally: + position = workbook_stream.tell() try: - workbook.close() + workbook_stream.seek(0) + workbook = openpyxl.load_workbook( + workbook_stream, data_only=True, read_only=True + ) except Exception: - pass + return try: - workbook_stream.seek(position) - except Exception: - pass + for worksheet in workbook.worksheets: + frame = sheets.get(worksheet.title) + if frame is None: + continue + object_cols: set[int] = set() + # pandas treats the first row as the header, so data row i lives in + # openpyxl row i + 2 (both 1-indexed vs 0-indexed and header offset). + for openpyxl_row in worksheet.iter_rows(min_row=2): + for cell in openpyxl_row: + value = cell.value + if ( + value is None + or isinstance(value, bool) + or not isinstance(value, (int, float)) + ): + continue + symbol = _currency_symbol(cell.number_format, value) + if symbol is None: + continue + data_row = cell.row - 2 + data_col = cell.column - 1 + if not (0 <= data_row < len(frame)) or not ( + 0 <= data_col < len(frame.columns) + ): + continue + text = str(frame.iat[data_row, data_col]) + if symbol in text: + continue + if data_col not in object_cols: + # A str label cannot live in a numeric column: widen it once. + frame[frame.columns[data_col]] = frame[ + frame.columns[data_col] + ].astype(object) + object_cols.add(data_col) + if _is_currency_position_prefix(str(cell.number_format), value): + text = f"{symbol}{text}" + else: + text = f"{text}{symbol}" + frame.iat[data_row, data_col] = text + finally: + try: + workbook.close() + except Exception: + pass + try: + workbook_stream.seek(position) + except Exception: + pass # Some producers write the legacy attribute "showZeroes" on , where the diff --git a/packages/markitdown/tests/test_xlsx_currency.py b/packages/markitdown/tests/test_xlsx_currency.py index b36d952d8..d4cb68473 100644 --- a/packages/markitdown/tests/test_xlsx_currency.py +++ b/packages/markitdown/tests/test_xlsx_currency.py @@ -63,7 +63,7 @@ def test_euro_suffix_format() -> None: sheet = workbook.active sheet.append(["Price"]) sheet.append([42]) - sheet["A2"].number_format = '#,##0.00 [$€-x-euro2]' + sheet["A2"].number_format = "#,##0.00 [$€-x-euro2]" stream = io.BytesIO() workbook.save(stream) workbook.close() From bd3566a4cf6efd291830283446fc7dbc4546d156 Mon Sep 17 00:00:00 2001 From: Islam El-Nashar Date: Fri, 25 Sep 2026 13:53:48 +0300 Subject: [PATCH 4/8] fix(xlsx): Sc-based currency detection, quote-aware sections, black formatting --- .../markitdown/converters/_xlsx_converter.py | 59 ++++++++++++++++--- 1 file changed, 50 insertions(+), 9 deletions(-) diff --git a/packages/markitdown/src/markitdown/converters/_xlsx_converter.py b/packages/markitdown/src/markitdown/converters/_xlsx_converter.py index d2ebae4c8..cbc8cd564 100644 --- a/packages/markitdown/src/markitdown/converters/_xlsx_converter.py +++ b/packages/markitdown/src/markitdown/converters/_xlsx_converter.py @@ -1,6 +1,7 @@ import io import re import sys +import unicodedata import zipfile from contextlib import contextmanager from typing import BinaryIO, Any, Iterator, Optional @@ -40,18 +41,58 @@ # Currency symbols that may appear in Excel number formats, either as quoted # literals (e.g. '"$"#,##0.00') or locale blocks (e.g. '[$€-x-euro2]'). # See https://github.com/microsoft/markitdown/issues/53. -_CURRENCY_SYMBOL_RE = re.compile(r"[$€£¥₹₽₩₪₺₴₸₫₦¤]") _LOCALE_BLOCK_RE = re.compile(r"\[\$([^\]-]+)") _QUOTED_LITERAL_RE = re.compile(r'"([^"]*)"') +def _find_currency_symbol(text: str) -> tuple[Optional[str], int]: + """Return the first Unicode currency symbol (category Sc) and its index. + + Detecting category Sc covers every currency sign (e.g. $, €, ฿, ₱) + instead of maintaining a partial hard-coded list. + """ + for index, char in enumerate(text): + if unicodedata.category(char) == "Sc": + return char, index + return None, -1 + + +def _split_format_sections(number_format: str) -> list[str]: + """Split an Excel number format on `;` separators. + + Separators inside quoted literals ("...") or escaped with a backslash + are part of the section text, not separators. + """ + parts: list[str] = [] + current: list[str] = [] + in_quotes = False + escaped = False + for char in number_format: + if escaped: + current.append(char) + escaped = False + elif char == "\\": + current.append(char) + escaped = True + elif char == '"': + in_quotes = not in_quotes + current.append(char) + elif char == ";" and not in_quotes: + parts.append("".join(current)) + current = [] + else: + current.append(char) + parts.append("".join(current)) + return parts + + def _select_format_section(number_format: str, value: Any) -> str: """Pick the `;`-separated Excel section that applies to `value`. Excel uses: 1 section = all numbers, 2 sections = positive+zero / negative, 3 sections = positive / negative / zero (4th section is text and ignored). """ - parts = number_format.split(";") + parts = _split_format_sections(number_format) if len(parts) == 1: return parts[0] if len(parts) == 2: @@ -84,15 +125,15 @@ def _currency_symbol(number_format: Any, value: Any = None) -> Optional[str]: section = ( _select_format_section(number_format, value) if value is not None - else number_format.split(";")[0] + else _split_format_sections(number_format)[0] ) quoted = "".join(_QUOTED_LITERAL_RE.findall(section)) locale = "".join(_LOCALE_BLOCK_RE.findall(section)) candidates = quoted + locale if not candidates: candidates = section - match = _CURRENCY_SYMBOL_RE.search(candidates) - return match.group(0) if match else None + symbol, _ = _find_currency_symbol(candidates) + return symbol def _is_currency_position_prefix(number_format: str, value: Any = None) -> bool: @@ -100,13 +141,13 @@ def _is_currency_position_prefix(number_format: str, value: Any = None) -> bool: section = ( _select_format_section(number_format, value) if value is not None - else number_format.split(";")[0] + else _split_format_sections(number_format)[0] ) - symbol_match = _CURRENCY_SYMBOL_RE.search(section) + symbol, position = _find_currency_symbol(section) placeholder_match = re.search(r"[#0?]", section) - if not symbol_match or not placeholder_match: + if symbol is None or not placeholder_match: return True - return symbol_match.start() < placeholder_match.start() + return position < placeholder_match.start() def _overlay_currency_labels(sheets: dict[str, Any], workbook_stream: BinaryIO) -> None: From d04760bbe10b43544d7365486f20762bdfa068b3 Mon Sep 17 00:00:00 2001 From: Islam El-Nashar Date: Fri, 25 Sep 2026 13:53:49 +0300 Subject: [PATCH 5/8] test(xlsx): fix tautological assertion, add baht and quoted-semicolon tests --- .../markitdown/tests/test_xlsx_currency.py | 21 +++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/packages/markitdown/tests/test_xlsx_currency.py b/packages/markitdown/tests/test_xlsx_currency.py index d4cb68473..d6677a565 100644 --- a/packages/markitdown/tests/test_xlsx_currency.py +++ b/packages/markitdown/tests/test_xlsx_currency.py @@ -8,7 +8,6 @@ from markitdown import StreamInfo from markitdown.converters import XlsxConverter - _INFO = StreamInfo(extension=".xlsx") @@ -105,4 +104,22 @@ def test_section_specific_currency_uses_cell_value() -> None: markdown = _convert(stream.getvalue()) assert "$10" in markdown assert "€" in markdown - assert "$-5" not in markdown.replace("$-5", "") or "€" in markdown + assert "$-5" not in markdown + + +def test_thai_baht_format() -> None: + workbook = openpyxl.Workbook() + sheet = workbook.active + sheet.append(["Price"]) + sheet.append([42]) + sheet["A2"].number_format = '"฿"#,##0' + stream = io.BytesIO() + workbook.save(stream) + workbook.close() + assert "฿42" in _convert(stream.getvalue()) + + +def test_quoted_semicolon_is_not_a_section_separator() -> None: + from markitdown.converters._xlsx_converter import _select_format_section + + assert _select_format_section('"$;gross"#,##0', -5) == '"$;gross"#,##0' From a9c769f54ccbc8cff0e4f7ef44b9bfffc4919a57 Mon Sep 17 00:00:00 2001 From: aslamalkarywk7 Date: Fri, 25 Sep 2026 14:03:07 +0000 Subject: [PATCH 6/8] fix(xlsx): resolve reviewer correctness blockers on currency overlay - Ignore locale-only blocks and system-symbol blocks; only literal locale payloads contribute display tokens. - Preserve full currency labels (multi-char and ISO codes) from quoted literals and locale blocks; keep bare-symbol fallback. - Evaluate explicit section conditions with first-match wins and unconditional fallback; sign rules unchanged otherwise. - Compute prefix/suffix placement from display text with metadata stripped but displayed tokens preserved. - Drive overlay iteration from pandas frame rows so stale declared dimensions cannot hide trailing data rows. - Add regression tests: locale-only, full labels, conditions, placement, stale dimension. Black clean, currency suite green. Signed-off-by: aslamalkarywk7 --- .../markitdown/converters/_xlsx_converter.py | 191 ++++++++++++++---- .../markitdown/tests/test_xlsx_currency.py | 114 +++++++++++ 2 files changed, 261 insertions(+), 44 deletions(-) diff --git a/packages/markitdown/src/markitdown/converters/_xlsx_converter.py b/packages/markitdown/src/markitdown/converters/_xlsx_converter.py index cbc8cd564..2fbff60b4 100644 --- a/packages/markitdown/src/markitdown/converters/_xlsx_converter.py +++ b/packages/markitdown/src/markitdown/converters/_xlsx_converter.py @@ -38,11 +38,21 @@ ACCEPTED_XLS_FILE_EXTENSIONS = [".xls"] -# Currency symbols that may appear in Excel number formats, either as quoted -# literals (e.g. '"$"#,##0.00') or locale blocks (e.g. '[$€-x-euro2]'). -# See https://github.com/microsoft/markitdown/issues/53. -_LOCALE_BLOCK_RE = re.compile(r"\[\$([^\]-]+)") -_QUOTED_LITERAL_RE = re.compile(r'"([^"]*)"') +# Bracketed number-format syntax that never renders as cell text, e.g. locale +# blocks ([$-409]), color tags ([Red], [Color10]) and explicit conditions +# ([>=100]). See https://github.com/microsoft/markitdown/issues/53. +_SQUARE_BLOCK_RE = re.compile(r"\[[^\]]*\]") +# Quoted literals (e.g. '"$"#,##0.00'), honoring backslash escapes. +_QUOTED_LITERAL_RE = re.compile(r'"((?:[^"\\]|\\.)*)"') +# Locale blocks that display a literal currency token (e.g. [$€-x-euro2] +# shows €, [$USD-409] shows USD). A payload starting with "-" ([$-409]) is +# locale metadata only, and "[$$-...]" names the system symbol, which cannot +# be determined statically, so neither contributes a token. +_LOCALE_CURRENCY_RE = re.compile(r"\[\$([^\]]*)\]") +# Leading explicit condition of a format section (e.g. [>=100]"$"0). +_CONDITION_RE = re.compile(r"^\s*\[(>=|<=|<>|>|<|=)\s*([^\]]+?)\s*\]") +# ISO 4217 currency codes are exactly three uppercase ASCII letters. +_CURRENCY_CODE_RE = re.compile(r"^[A-Z]{3}$") def _find_currency_symbol(text: str) -> tuple[Optional[str], int]: @@ -86,39 +96,126 @@ def _split_format_sections(number_format: str) -> list[str]: return parts +def _strip_square_blocks(section: str) -> str: + """Remove bracketed metadata (locale/color/condition blocks).""" + return _SQUARE_BLOCK_RE.sub("", section) + + +def _display_text(section: str) -> str: + """Section with metadata removed but displayed tokens preserved. + + Locale blocks carrying a literal token ([$USD-409] shows USD) are + replaced by that token; locale-only blocks ([$-409]), system-symbol + blocks ([$$-...]) and color/condition blocks are removed. + """ + return _SQUARE_BLOCK_RE.sub( + "", _LOCALE_CURRENCY_RE.sub(_locale_block_replacement, section) + ) + + +def _locale_block_replacement(match: re.Match[str]) -> str: + payload = match.group(1) + if payload.startswith("-") or payload.startswith("$"): + return "" + return payload.split("-", 1)[0].strip() + + +def _unescape_literal(text: str) -> str: + """Collapse backslash escapes inside a quoted literal.""" + return re.sub(r"\\(.)", r"\1", text) + + def _select_format_section(number_format: str, value: Any) -> str: """Pick the `;`-separated Excel section that applies to `value`. - Excel uses: 1 section = all numbers, 2 sections = positive+zero / negative, - 3 sections = positive / negative / zero (4th section is text and ignored). + Without explicit conditions Excel uses: 1 section = all numbers, + 2 sections = positive+zero / negative, 3 sections = positive / negative + / zero (a 4th text section is ignored). Explicit conditions such as + [>=100] override the sign rules: the first matching section wins and an + unconditional section acts as the fallback for values reaching it. """ parts = _split_format_sections(number_format) - if len(parts) == 1: + if value is None or len(parts) == 1: return parts[0] - if len(parts) == 2: - # Zero renders with the first section when only two are present. - try: - is_negative = float(value) < 0 - except Exception: - is_negative = False - return parts[1] if is_negative else parts[0] try: numeric = float(value) except Exception: return parts[0] - if numeric > 0: - return parts[0] - if numeric < 0: - return parts[1] - return parts[2] + if not any(_CONDITION_RE.match(part) for part in parts): + if len(parts) == 2: + # Zero renders with the first section when only two are present. + return parts[1] if numeric < 0 else parts[0] + if numeric > 0: + return parts[0] + if numeric < 0: + return parts[1] + return parts[2] + for part in parts: + match = _CONDITION_RE.match(part) + if match is None: + return part + if _condition_matches(match.group(1), match.group(2), numeric): + return part + return parts[0] + + +def _condition_matches(operator: str, target: str, value: float) -> bool: + """Evaluate an explicit section condition such as [>=100].""" + try: + bound = float(target) + except ValueError: + return False + if operator == ">=": + return value >= bound + if operator == "<=": + return value <= bound + if operator == "<>": + return value != bound + if operator == ">": + return value > bound + if operator == "<": + return value < bound + return value == bound + + +def _display_currency_tokens(section: str) -> list[str]: + """Currency labels a format section actually displays, in order. + + Quoted literals keep their full label ("R$", not "$"); locale blocks + contribute their literal payload ([$USD-409] shows USD) while + locale-only blocks ([$-409]) and system-symbol blocks ([$$-...]) + contribute nothing; anything else must be a bare currency symbol. + """ + tokens: list[str] = [] + for raw in _QUOTED_LITERAL_RE.findall(section): + literal = _unescape_literal(raw).strip() + if not literal: + continue + symbol, _ = _find_currency_symbol(literal) + if symbol is not None: + tokens.append(literal) + elif _CURRENCY_CODE_RE.match(literal): + tokens.append(literal) + for match in _LOCALE_CURRENCY_RE.finditer(section): + payload = match.group(1) + if payload.startswith("-") or payload.startswith("$"): + continue + token = payload.split("-", 1)[0].strip() + if token: + tokens.append(token) + display = _QUOTED_LITERAL_RE.sub("", _strip_square_blocks(section)) + symbol, _ = _find_currency_symbol(display) + if symbol is not None: + tokens.append(symbol) + return tokens def _currency_symbol(number_format: Any, value: Any = None) -> Optional[str]: - """Return the currency symbol of an Excel number format, if it has one. + """Return the currency label of an Excel number format, if it has one. - When `value` is given, the `;`-separated section matching its sign is - inspected (positive / negative / zero). Otherwise the first section is - used, matching how positive values are rendered. + When `value` is given, the `;`-separated section matching it (by sign, + or by explicit conditions such as [>=100]) is inspected. Otherwise the + first section is used, matching how positive values are rendered. """ if not isinstance(number_format, str): return None @@ -127,25 +224,31 @@ def _currency_symbol(number_format: Any, value: Any = None) -> Optional[str]: if value is not None else _split_format_sections(number_format)[0] ) - quoted = "".join(_QUOTED_LITERAL_RE.findall(section)) - locale = "".join(_LOCALE_BLOCK_RE.findall(section)) - candidates = quoted + locale - if not candidates: - candidates = section - symbol, _ = _find_currency_symbol(candidates) - return symbol + tokens = _display_currency_tokens(section) + return tokens[0] if tokens else None def _is_currency_position_prefix(number_format: str, value: Any = None) -> bool: - """Decide whether the currency symbol renders before the value.""" + """Decide whether the currency label renders before the value. + + Placement is computed from the displayed label and numeric placeholders + in the metadata-stripped section, so bracketed blocks ([Color10], + [$-409], [>=100]) cannot shift either search. + """ + if not isinstance(number_format, str): + return True section = ( _select_format_section(number_format, value) if value is not None else _split_format_sections(number_format)[0] ) - symbol, position = _find_currency_symbol(section) - placeholder_match = re.search(r"[#0?]", section) - if symbol is None or not placeholder_match: + tokens = _display_currency_tokens(section) + if not tokens: + return True + display = _display_text(section) + position = display.find(tokens[0]) + placeholder_match = re.search(r"[#0?]", display) + if position == -1 or placeholder_match is None: return True return position < placeholder_match.start() @@ -163,9 +266,7 @@ def _overlay_currency_labels(sheets: dict[str, Any], workbook_stream: BinaryIO) position = workbook_stream.tell() try: workbook_stream.seek(0) - workbook = openpyxl.load_workbook( - workbook_stream, data_only=True, read_only=True - ) + workbook = openpyxl.load_workbook(workbook_stream, data_only=True) except Exception: return try: @@ -174,10 +275,14 @@ def _overlay_currency_labels(sheets: dict[str, Any], workbook_stream: BinaryIO) if frame is None: continue object_cols: set[int] = set() - # pandas treats the first row as the header, so data row i lives in - # openpyxl row i + 2 (both 1-indexed vs 0-indexed and header offset). - for openpyxl_row in worksheet.iter_rows(min_row=2): - for cell in openpyxl_row: + # Drive iteration from the rows pandas actually read: the + # declared worksheet dimension can be stale (hiding trailing + # rows from iter_rows()) while pandas still returns them. + # pandas treats the first row as the header, so data row i + # lives in openpyxl row i + 2. + for data_row in range(len(frame)): + for data_col in range(len(frame.columns)): + cell = worksheet.cell(row=data_row + 2, column=data_col + 1) value = cell.value if ( value is None @@ -188,8 +293,6 @@ def _overlay_currency_labels(sheets: dict[str, Any], workbook_stream: BinaryIO) symbol = _currency_symbol(cell.number_format, value) if symbol is None: continue - data_row = cell.row - 2 - data_col = cell.column - 1 if not (0 <= data_row < len(frame)) or not ( 0 <= data_col < len(frame.columns) ): diff --git a/packages/markitdown/tests/test_xlsx_currency.py b/packages/markitdown/tests/test_xlsx_currency.py index d6677a565..2d16218a8 100644 --- a/packages/markitdown/tests/test_xlsx_currency.py +++ b/packages/markitdown/tests/test_xlsx_currency.py @@ -1,6 +1,8 @@ """Currency-formatted Excel cells keep their label (microsoft/markitdown#53).""" import io +import re +import zipfile import openpyxl import pytest @@ -10,6 +12,8 @@ _INFO = StreamInfo(extension=".xlsx") +_DIMENSION_REF_RE = re.compile(rb'( bytes: workbook = openpyxl.Workbook() @@ -123,3 +127,113 @@ def test_quoted_semicolon_is_not_a_section_separator() -> None: from markitdown.converters._xlsx_converter import _select_format_section assert _select_format_section('"$;gross"#,##0', -5) == '"$;gross"#,##0' + + +def test_locale_only_block_produces_no_label() -> None: + from markitdown.converters._xlsx_converter import _currency_symbol + + assert _currency_symbol("[$-409]#,##0.00", 42) is None + assert _currency_symbol("[$-409]0%", 0.42) is None + + workbook = openpyxl.Workbook() + sheet = workbook.active + sheet.append(["Price", "Rate"]) + sheet.append([42, 0.42]) + sheet["A2"].number_format = "[$-409]#,##0.00" + sheet["B2"].number_format = "[$-409]0%" + stream = io.BytesIO() + workbook.save(stream) + workbook.close() + markdown = _convert(stream.getvalue()) + assert "$" not in markdown + + +def test_full_currency_labels_are_preserved() -> None: + from markitdown.converters._xlsx_converter import _currency_symbol + + assert _currency_symbol('"R$" #,##0.00', 42) == "R$" + assert _currency_symbol("[$A$-en-AU]#,##0.00", 42) == "A$" + assert _currency_symbol("[$USD-409]#,##0.00", 42) == "USD" + assert _currency_symbol('#,##0.00 "CHF"', 42) == "CHF" + assert _currency_symbol('$0" net"', 5) == "$" + + workbook = openpyxl.Workbook() + sheet = workbook.active + sheet.append(["BR", "CH"]) + sheet.append([42, 42]) + sheet["A2"].number_format = '"R$" #,##0.00' + sheet["B2"].number_format = '#,##0.00 "CHF"' + stream = io.BytesIO() + workbook.save(stream) + workbook.close() + markdown = _convert(stream.getvalue()) + assert "R$42" in markdown + assert "42CHF" in markdown + + +def test_conditional_sections_select_by_condition() -> None: + from markitdown.converters._xlsx_converter import ( + _currency_symbol, + _select_format_section, + ) + + fmt = '[>=100]"$"0;"€"0' + assert _select_format_section(fmt, 50) == '"€"0' + assert _select_format_section(fmt, 150) == '[>=100]"$"0' + assert _currency_symbol(fmt, 50) == "€" + assert _currency_symbol(fmt, 150) == "$" + assert _currency_symbol('[>=100]"$"0;0', 50) is None + + workbook = openpyxl.Workbook() + sheet = workbook.active + sheet.append(["Amount"]) + sheet.append([50]) + sheet.append([150]) + sheet["A2"].number_format = fmt + sheet["A3"].number_format = fmt + stream = io.BytesIO() + workbook.save(stream) + workbook.close() + markdown = _convert(stream.getvalue()) + assert "€50" in markdown + assert "$150" in markdown + + +def test_placement_ignores_bracketed_metadata() -> None: + from markitdown.converters._xlsx_converter import ( + _is_currency_position_prefix, + ) + + assert _is_currency_position_prefix('[Color10]"$"#,##0', 150) is True + assert _is_currency_position_prefix('[$-409]#,##0.00"€"', 42) is False + assert _is_currency_position_prefix('$0" net"', 5) is True + assert _is_currency_position_prefix("#,##0.00 [$€-x-euro2]", 42) is False + + +def test_stale_dimension_still_labels_trailing_rows() -> None: + workbook = openpyxl.Workbook() + sheet = workbook.active + sheet.append(["Price"]) + sheet.append([10]) + sheet.append([20]) + sheet["A2"].number_format = '"$"#,##0' + sheet["A3"].number_format = '"$"#,##0' + stream = io.BytesIO() + workbook.save(stream) + workbook.close() + + # Simulate a producer with a stale declared dimension covering only the + # first data row while actual data runs through row 3. + source = zipfile.ZipFile(io.BytesIO(stream.getvalue())) + patched = io.BytesIO() + with zipfile.ZipFile(patched, "w", zipfile.ZIP_DEFLATED) as target: + for item in source.infolist(): + data = source.read(item.filename) + if item.filename == "xl/worksheets/sheet1.xml": + data = _DIMENSION_REF_RE.sub(rb"\1A1:A2\2", data) + target.writestr(item, data) + source.close() + + markdown = _convert(patched.getvalue()) + assert "$10" in markdown + assert "$20" in markdown From 3075edf405c4e5de26002866c0412e2112b2e7cb Mon Sep 17 00:00:00 2001 From: aslamalkarywk7 Date: Sat, 26 Sep 2026 06:23:27 +0000 Subject: [PATCH 7/8] fix(xlsx): ignore literal digits when placing currency labels Excel treats quoted text and backslash-escaped chars as literals, not numeric placeholders. Mask them for placeholder search so quoted-zero format with value 5 yields prefix placement not suffix. Addresses reviewer blocker on microsoft/markitdown#2538. --- .../src/markitdown/converters/_xlsx_converter.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/markitdown/src/markitdown/converters/_xlsx_converter.py b/packages/markitdown/src/markitdown/converters/_xlsx_converter.py index 2fbff60b4..6c9f7cd35 100644 --- a/packages/markitdown/src/markitdown/converters/_xlsx_converter.py +++ b/packages/markitdown/src/markitdown/converters/_xlsx_converter.py @@ -232,8 +232,8 @@ def _is_currency_position_prefix(number_format: str, value: Any = None) -> bool: """Decide whether the currency label renders before the value. Placement is computed from the displayed label and numeric placeholders - in the metadata-stripped section, so bracketed blocks ([Color10], - [$-409], [>=100]) cannot shift either search. + in the metadata-stripped section. Quoted and escaped literal digits are + excluded from the numeric placeholder search. """ if not isinstance(number_format, str): return True @@ -247,7 +247,11 @@ def _is_currency_position_prefix(number_format: str, value: Any = None) -> bool: return True display = _display_text(section) position = display.find(tokens[0]) - placeholder_match = re.search(r"[#0?]", display) + placeholders = _QUOTED_LITERAL_RE.sub( + lambda match: " " * len(match.group(0)), display + ) + placeholders = re.sub(r"\\.", lambda match: " " * len(match.group(0)), placeholders) + placeholder_match = re.search(r"[#0?]", placeholders) if position == -1 or placeholder_match is None: return True return position < placeholder_match.start() From f137a6cc46979cccae4a1538c85a323d5f34e30e Mon Sep 17 00:00:00 2001 From: aslamalkarywk7 Date: Sat, 26 Sep 2026 11:33:51 +0000 Subject: [PATCH 8/8] test(xlsx): pin quoted and escaped literal-digit placement Add regressions for literal-digit masking requested in review. --- .../markitdown/tests/test_xlsx_currency.py | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/packages/markitdown/tests/test_xlsx_currency.py b/packages/markitdown/tests/test_xlsx_currency.py index 2d16218a8..10a47552f 100644 --- a/packages/markitdown/tests/test_xlsx_currency.py +++ b/packages/markitdown/tests/test_xlsx_currency.py @@ -210,6 +210,50 @@ def test_placement_ignores_bracketed_metadata() -> None: assert _is_currency_position_prefix("#,##0.00 [$€-x-euro2]", 42) is False +def test_quoted_literal_digit_is_not_placeholder() -> None: + from markitdown.converters._xlsx_converter import ( + _currency_symbol, + _is_currency_position_prefix, + ) + + fmt = '"0 $"0' + assert _currency_symbol(fmt, 5) == "0 $" + assert _is_currency_position_prefix(fmt, 5) is True + + workbook = openpyxl.Workbook() + sheet = workbook.active + sheet.append(["Price"]) + sheet.append([5]) + sheet["A2"].number_format = fmt + stream = io.BytesIO() + workbook.save(stream) + workbook.close() + markdown = _convert(stream.getvalue()) + assert "0 $5" in markdown + assert "50 $" not in markdown + + +def test_escaped_literal_digit_is_not_placeholder() -> None: + from markitdown.converters._xlsx_converter import ( + _is_currency_position_prefix, + ) + + fmt = r'\0"$"0' + assert _is_currency_position_prefix(fmt, 5) is True + + workbook = openpyxl.Workbook() + sheet = workbook.active + sheet.append(["Price"]) + sheet.append([5]) + sheet["A2"].number_format = fmt + stream = io.BytesIO() + workbook.save(stream) + workbook.close() + markdown = _convert(stream.getvalue()) + assert "$5" in markdown + assert "5$" not in markdown + + def test_stale_dimension_still_labels_trailing_rows() -> None: workbook = openpyxl.Workbook() sheet = workbook.active