ROB: Clarify CMYK comment and detect image format from Pillow instead of guessing - #3902
ROB: Clarify CMYK comment and detect image format from Pillow instead of guessing#3902BiswasNehaa wants to merge 5 commits into
Conversation
BiswasNehaa
changed the title
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3902 +/- ##
==========================================
- Coverage 97.93% 97.91% -0.02%
==========================================
Files 57 57
Lines 11002 11003 +1
Branches 2058 2058
==========================================
- Hits 10775 10774 -1
- Misses 126 128 +2
Partials 101 101 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
6f2f50c to
81c4851
Compare
| # (in which case it is interpreted as raw, undecoded pixel data). | ||
| try: | ||
| img = Image.open(BytesIO(data), formats=("TIFF", "PNG")) | ||
| image_format = cast(str, img.format) |
There was a problem hiding this comment.
These lines are never executed, thus either indicating an issue in your logic or incomplete test data. Please check.
There was a problem hiding this comment.
Found it — a bad merge conflict resolution had dropped the entire except UnidentifiedImageError: body, which caused a syntax error and explains both the coverage gap and the failing CI. Fixed and pushed; the fallback logic is restored and tests pass locally now.
There was a problem hiding this comment.
We still got two missing lines in coverage, which indicates something is still going wrong (although the Codecov details page is unable to show any file details at the moment, which seems to be a recurring problem on their side).
| # (in which case it is interpreted as raw, undecoded pixel data). | ||
| try: | ||
| img = Image.open(BytesIO(data), formats=("TIFF", "PNG")) | ||
| image_format = cast(str, img.format) |
There was a problem hiding this comment.
We still got two missing lines in coverage, which indicates something is still going wrong (although the Codecov details page is unable to show any file details at the moment, which seems to be a recurring problem on their side).
2 participants
This covers two of three items in #3869 (CMYK comment + LZW/ASCII85 logic ). Skipped the ICC one.
CMYK: The old comment there pointed to a closed pair with no information. Pillow only got Native CMYK Palette support in v10.1.0, pypdf supports back to 8.0 .0 so the work around stays.
LZW/ASCII85: switched to opening the image first and trusting Pillow's actual detected format, instead of guessing from the filter type ...this is what j-t-1 suggested in the issue thread. Kept the old guess as a fallback for when Pillow can't read it at all.
tested: ran the image test suite, same pass rate as before my change; checked the two real fixture PDFs that hit this code give identical output before/after.
Used Claude to help investigate the Pillow version history and verify the diff.