Skip to content

fix(epub): forward conversion options to the HTML converter - #2426

Merged
afourney merged 2 commits into
microsoft:mainfrom
kevin9327:fix/epub-converter-options
Sep 9, 2026
Merged

fix(epub): forward conversion options to the HTML converter#2426
afourney merged 2 commits into
microsoft:mainfrom
kevin9327:fix/epub-converter-options

Conversation

@kevin9327

Copy link
Copy Markdown
Contributor

What this changes

Conversion options are ignored for EPUB. keep_data_uris=True keeps the image source for
HTML, DOCX, XLSX and PPTX, but not for the same image inside an EPUB chapter:

.html  ->  ![diagram](data:image/png;base64,iVBORw0KGgoAAAANSUhEUg==)
.epub  ->  ![diagram](data:image/png;base64...)

Why

EpubConverter.convert accepts **kwargs and then calls the HTML converter without them:

converted_content = self._html_converter.convert(
    f,
    StreamInfo(
        mimetype=mimetype,
        extension=extension,
        filename=filename,
    ),
)

Every other converter that renders through HtmlConverter passes the bag along:

# _docx_converter.py
return self._html_converter.convert_string(html_result, **kwargs)

# _xlsx_converter.py
self._html_converter.convert_string(html_content, **kwargs)

# _pptx_converter.py
self._html_converter.convert_string(html_table, **kwargs).markdown.strip()

keep_data_uris is the option with a visible effect today -- it is a CLI flag
(--keep-data-uris) and reaches the converter through convert_stream/convert -- but the
gap covers everything the caller passes, including the markdownify options
(heading_style, bullets, ...) and strict.

Fix

Forward **kwargs, exactly as the sibling converters do. One line.

Tests

Added to packages/markitdown/tests/test_module_misc.py, over a minimal in-memory EPUB whose
one chapter carries a data-URI image:

  • keep_data_uris=True keeps the full data URI
  • pinned: without the option the URI is still truncated to data:image/png;base64..., and
    the payload does not appear
  • pinned: metadata, title and chapter text are unchanged

Against unmodified main:

E  AssertionError: assert '![diagram](data:image/png;base64,iVBORw0KGgoAAAANSUhEUg==)' in
   '**Title:** Example book\n\nChapter text.\n\n![diagram](data:image/png;base64...)'
1 failed, 3 passed, 65 deselected

With the fix:

4 passed, 65 deselected

How I tested

Windows 11, Python 3.12, editable install of packages/markitdown[all].

pytest tests/test_module_misc.py -k epub   ->  1 failed, 3 passed  (before)
pytest tests/test_module_misc.py -k epub   ->  4 passed            (after)
pytest tests/                              ->  18 failed, 429 passed, 4 skipped

Those 18 are unchanged by this PR: main gives 18 failed, 426 passed, 4 skipped on this
machine before any edit, and the 3 added tests account for the difference. They are the CLI
stdout-encoding, Windows file-URI and speech-transcription tests that need a UTF-8 console, a
case-sensitive path and network/ffmpeg.

black --check clean on both files.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The focused fix matches sibling converters and has appropriate regression coverage.

Pull request overview

Forwards conversion options from EPUB processing to the underlying HTML converter.

Changes:

  • Passes **kwargs when converting EPUB chapters.
  • Adds regression coverage for data-URI handling and unchanged metadata/text.
File summaries
File Description
packages/markitdown/src/markitdown/converters/_epub_converter.py Forwards conversion options.
packages/markitdown/tests/test_module_misc.py Adds in-memory EPUB regression tests.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@afourney
afourney merged commit 5dc7baf into microsoft:main Sep 9, 2026
10 checks passed
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.

3 participants