Repository navigation
FIX: apply font_color to PDFs PDFConverter generates - #3136
Utkarsh Bahuguna (u7k4rs6) wants to merge 3 commits into
Conversation
| reader = PdfReader(pdf_path) | ||
| return [ | ||
| { | ||
| tuple(round(float(channel) * 255) for channel in match) |
There was a problem hiding this comment.
🟡 Should Fix: This new helper fails the project's type check. uv run --no-sync ty check pyrit\converter\pdf_converter.py tests\unit\converter\test_pdf_converter.py reports invalid-return-type: tuple(...) has type tuple[int, ...], but the return annotation requires exactly three channels (tuple[int, int, int]).
Please unpack each match and build an explicit three-element tuple:
(
round(float(red) * 255),
round(float(green) * 255),
round(float(blue) * 255),
)
for red, green, blue in _FILL_COLOR_OP.findall(...)That preserves the RGB contract and removes this error. The documented make ty target includes unit tests, so passing pytest alone does not catch it.
There was a problem hiding this comment.
Done in 1dc9c00, now it unpacks the three channels so the helper returns a real tuple[int, int, int]. ty is clean on both files, and I've started running it on the test files too.
Fixes #3105.
Went with the option that doesn't change anything for people who don't pass a color:
font_colornow defaults toNone.font_color, or black when it isn't set (same as today), and set it again after each page break.font_colorisn't set, so hidden injected text keeps working. Iffont_coloris set, they use it.font_coloris in the identifier when set. Unset is dropped, so default PDFConverter hashes don't move. I leftrow_height/column_width/injection_itemsout to keep this small.Tests go through
convert_asyncand read the fill-color ops back out of the PDF content stream:The new ones fail on main. tests/unit/converter, registry and backend/test_converter_service: 2928 passed.