[usmanmateen]

Resolves #4677

_find_images_in_sheet read pictures with img.get_data(), which odfdo's DrawImage doesn't have, so every picture anchored to an ODS sheet was dropped with only a debug log. This reads the pictures in <table:shapes> and builds them with _image_ref_from_odf_image, like the other ODF pictures. Pictures anchored to a cell are left to the rich cell path, so they aren't added twice.

#4557 had a similar change (it skipped pictures with a table-cell ancestor), but it was merged into a stacked branch that didn't reach main.

Checklist:

• Documentation has been updated, if necessary. (not needed)
• Examples have been added, if necessary. (not needed)
• Tests have been added, if necessary.

Review evidence:

• The reproduction from ODS: pictures anchored to the sheet are dropped #4677 prints ConversionStatus.SUCCESS 1 on main and ConversionStatus.SUCCESS 2 with this change.
• test_ods_sheet_anchored_picture_is_kept builds a sheet with one picture in a cell and one in <table:shapes> and expects 2 pictures. It fails on main with 1 (the sheet picture is dropped). A version that reads every picture of the table fails with 3 (the cell picture is added twice).
• pytest tests/test_backend_opendocument.py: 56 passed (55 on main plus the new test), with the same 3 warnings as main.
• make validate: all hooks pass.

[github-actions]

✅ DCO Check Passed

Thanks @usmanmateen, all your commits are properly signed off. 🎉

[mergify]

Merge Protections

🟢 Merge protection satisfied — ready to merge.

Show 1 satisfied protection

🟢 Enforce conventional commit

Make sure that we follow https://www.conventionalcommits.org/en/v1.0.0/

• title ~= ^(fix|feat|docs|style|refactor|perf|test|build|ci|chore|revert)(?:\(.+\))?(!)?:

[github-actions]

AI triage (advisory)

Checked commit e4617b89b8. The analysis read the diff only. It did not run code from this pull request.

The PR changes `_find_images_in_sheet` in the ODS backend. It now reads pictures from `<table:shapes>` and builds them with `_image_ref_from_odf_image`. Before, `img.get_data()` failed and the code dropped these pictures. The PR adds two tests.

CI safety: no concern found. A maintainer must still approve CI.

2 related pull request(s) in the same code

PR	State	Signal	Assessment
#4666	merged	also references issue #4677	related: PR 4666 is merged. It makes `_image_ref_from_odf_image` convert CMYK pictures. It does not change `_find_images_in_sheet`, so the two changes do different things and do not conflict.
#4557	merged	referenced as #4557	related: PR 4557 is a broad typing and contract change. One part of it also changes `_find_images_in_sheet`. That part skips pictures with a cell ancestor and uses `_image_ref_from_odf_image`. The PR text says 4557 did not reach `main`. The base file still has the old `img.get_data()` code, so this PR does not duplicate a change on `main`.

_{The ai:* labels are advisory. They do not approve CI and they do not add tests:full. A new push runs this check again.}

[github-actions]

AI first review (advisory): no blocking issue found

This review covers the full diff at cd00e88524. It read the code only. It did not run tests or code from this pull request.

The PR fixes a bug where pictures anchored to an ODS sheet (stored in <table:shapes>) were always dropped. The old code called table.get_images() which returned images across the whole table, then called .get_data() on each — a method that does not exist on DrawImage objects — so every picture was silently discarded. The fix queries only <table:shapes>//draw:image and processes each image through the existing _image_ref_from_odf_image helper, which handles the data-loading and format-normalization correctly. A new test builds a sheet with one cell picture and one sheet-anchored picture and asserts that exactly two pictures appear in the converted document.

_{A maintainer makes the merge decision. This review never approves the pull request.}

[usmanmateen]

cc @PeterStaar-IBM: this is the _find_images_in_sheet follow-up from #4666. Happy to adjust anything.

[github-actions]

AI first review (advisory): no blocking issue found

This review covers the changes since cd00e88524 at 0e5f8c7edf. It read the code only. It did not run tests or code from this pull request.

This PR fixes ODS sheet-anchored pictures being silently dropped by replacing table.get_images() / img.get_data() with table.get_elements("table:shapes//draw:image") and the existing _image_ref_from_odf_image helper, matching how other ODF pictures are handled. The incremental update adds one assertion (assert all(p.image is not None for p in pictures)) to the new regression test, which directly resolves the only open finding from the previous review round. No new issues are introduced.

_{A maintainer makes the merge decision. This review never approves the pull request.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[dolfim-ibm]

lgtm