Skip to content

Fix DOCX equation prescript conversion - #2685

Open
Bicheng (Kenneth) (BichengWang) wants to merge 1 commit into
microsoft:mainfrom
BichengWang:BichengWang/contribution-markitdown-pr-20261009
Open

Bicheng (Kenneth) (BichengWang) wants to merge 1 commit into
microsoft:mainfrom
BichengWang:BichengWang/contribution-markitdown-pr-20261009

Conversation

@BichengWang

Copy link
Copy Markdown

DOCX equations containing m:sPre silently lose the entire prescript expression, including its base. In the new fixture, carbon isotope notation becomes Carbon isotope: $$. and the block uranium equation becomes $$X+$$.

Register and implement the prescript handler so it retains both scripts and the base, omits empty scripts, and attaches left scripts to an empty LaTeX atom. Regression coverage includes optional properties, empty scripts, a nested fraction, and a real DOCX fixture exercised through the shared library and CLI vectors.

Local validation on macOS with Python 3.12.15:

  • All 10 focused prescript regressions fail with the upstream converter and pass with the fix.
  • Full core Hatch suite: 1,069 passed, 59 skipped under existing skip conditions.
  • OCR suite: 108 passed; MCP suite: 21 passed.
  • Root pre-commit, diff whitespace checks, and regenerated fixture XML comparison passed.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Community review, so this does not clear the merge gate. I read the whole diff and ran it.

What I ran, on Windows with Python 3.12 against markitdown main (4cc9fa1) and this PR head (2f6df2d):

  • Built minimal docx files with m:sPre and converted them with MarkItDown().convert_stream(..., escape_underscores=False).
  • On main every sPre expression is dropped, base included: a Word-style sPre (with sPrePr/ctrlPr, sub 6, sup 14, base Fe) gives $$, and a followed by an sPre with no m:e gives $a$.
  • On this PR the same inputs give ${}_{6}^{14}{Fe}$ and $a{}_{6}^{14}{}$.
  • An empty <m:sub/> with sup 14 and base C gives ${}^{14}{C}$, so empty scripts are omitted as the PR says.
  • A nested sPre inside the base of another sPre gives ${}_{1}^{2}{{}_{3}^{4}{X}}$, and an sPre followed by an sSup gives ${}_{6}^{14}{C}x^{2}$. Both look right.
  • test_docx.py passes on the PR checkout (110 passed, 1 skipped).
  • The new docx fixture passes the vector tests in test_module_vectors.py and test_cli_vectors.py (the local-file, stream, file URI and data URI variants). The two http/url variants failed in my environment and I did not investigate them. They do not touch the math code.

Reading do_spre: it uses process_children_dict, so the sPrePr element becomes a Pr object under its own key and is ignored, and the {} prefix is what keeps the scripts attached to the left of the base. Registering "sPre": do_spre in the tag table is what makes it reachable, since main had a do_spre that returned None and was never registered.

One note, not a blocker: with the default escape_underscores=True the output is ${}\_{6}^{14}{C}$, which is how the existing math output already behaves for subscripts, and the new vector in _test_vectors.py expects that form. I did not change or test that behavior.

I did not run the full suite to completion. My full-suite run stopped at an unrelated Windows console-encoding failure on a Japanese CSV vector, so I only ran the files above.

I found no problems in the change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants