feat(markdown): include table image in Markdown export when available#680
feat(markdown): include table image in Markdown export when available#680maxmilian wants to merge 1 commit into
Conversation
|
✅ DCO Check Passed Thanks @maxmilian, all your commits are properly signed off. 🎉 |
Merge Protections🔴 1 of 2 protections blocking · waiting on 👀 reviews
🔴 Require two reviewer for test updatesWaiting for
This rule is failing.When test data is updated, we require two reviewers
Show 1 satisfied protection🟢 Enforce conventional commitMake sure that we follow https://www.conventionalcommits.org/en/v1.0.0/
|
e1a0d20 to
c3f2ad9
Compare
|
Rebased and force-pushed to resolve the conflict. It turned out to be more than a textual conflict: #664 split the monolithic
The behavior and public surface are unchanged from the original PR. Verified on the new base: |
MarkdownTableSerializer previously ignored TableItem.image entirely, only emitting the text grid, unlike MarkdownPictureSerializer which honors image_mode for pictures. Extract the shared image-emission logic into a module-level _serialize_image_part() usable by both serializers, and move _image_to_base64 up from PictureItem to the shared FloatingItem base so TableItem can call it too. MarkdownTableSerializer now appends the table image alongside the text table when item.image is set and image_mode is not PLACEHOLDER. Tables without a generated image (the common case) see no output change. Reworked onto the post-docling-project#664 module split (items/node.py, items/picture, transforms/serializer/markdown) after the original branch went stale. Closes docling-project#474 Signed-off-by: Max Hsu <maxmilian@gmail.com>
c3f2ad9 to
b361bb7
Compare
Closes #474.
What
MarkdownTableSerializeronly ever emitted the text/HTML table grid —TableItem.imagewas ignored entirely, unlikeMarkdownPictureSerializer, which already honorsimage_mode(PLACEHOLDER/EMBEDDED/REFERENCED) for pictures.Change
MarkdownPictureSerializer._serialize_image_part) into a module-level function reused by both the picture and table serializers — no behavior change to the existing picture path (verified byte-for-byte identical branches)._image_to_base64fromPictureItemup to the sharedFloatingItembase soTableItem(which is also aFloatingItem) can use it.MarkdownTableSerializer.serializenow appends the table image alongside the existing text table whenitem.imageis set andimage_modeis notPLACEHOLDER— the text table is preserved so this is purely additive.Design choice (open in the issue, unconfirmed by a maintainer)
I'd asked in the issue whether the image should replace or be emitted alongside the text table, and whether it should reuse
image_modeor a new dedicated option. I went with alongside + reuse existingimage_mode, since:Happy to adjust if a maintainer prefers different behavior.
Backward compatibility
Tables without a generated image (
item.image is None— the common case, since table images require thegenerate_table_imagespipeline option) see zero output change, even ifimage_modeis explicitly set to something other thanPLACEHOLDER. Covered by a dedicated regression test.Testing
Added 3 new tests in
test/test_serialization.py:test_md_table_with_image_embedded—EMBEDDEDmode, image presenttest_md_table_with_image_referenced—REFERENCEDmode, image present (asserts exact output)test_md_table_without_image_unaffected_by_image_mode— no image present, asserts byte-exact no-op regardless ofimage_modeFull suite:
uv run pytest -q→ 545 passed, 6 skipped (unrelated to this change).ruff check/ruff format --check/mypy docling_core testall clean.