From 212b28e602abe05e928f36d9e1d21a065f585c57 Mon Sep 17 00:00:00 2001 From: "James R. Barlow" Date: Tue, 7 Jul 2026 00:44:56 -0700 Subject: [PATCH] Fix mypy pikepdf Object typing errors in pdfinfo/optimize/graft Resolves the remaining 22 pikepdf-related mypy errors in pdfinfo/_image.py, pdfinfo/info.py, optimize.py, and _graft.py (89 -> 27 errors remaining, all in the concurrency/fpdf_renderer clusters). Adds pikepdf_get_int()/pikepdf_get_bool() to helpers.py: safe accessors for dict.get(key, default) results, whose static type is the ambiguous `Object | int`/`Object | bool` and doesn't support arithmetic/comparison against a plain int/bool. Important correctness fix caught by the test suite: pikepdf only returns pikepdf.Object wrappers under explicit_conversion() mode, which this codebase never enables. By default (implicit mode), PDF Integers/Booleans are already unboxed to native int/bool by the time callers see them, so calling the `.as_int()`/`.as_bool()`/ `.as_decimal()` safe accessors unconditionally crashes with AttributeError on the native-type case (test_oversized_page caught this for UserUnit). Fixed by using int()/float() builtins, which work polymorphically on both native numbers and pikepdf.Object (bool() does not, so pikepdf_get_bool checks isinstance first). Also: - pdfinfo/info.py: pass page.obj (an Object) to _process_content_streams() instead of page (a Page wrapper, not an Object subtype). - pdfinfo/_image.py: cast() around Matrix(array_object) - pikepdf's stub omits the Object/Array constructor overload that the C++ implementation actually supports. - optimize.py: cast() around Object.write()'s filter/decode_parms args for the same reason. - _graft.py: iterate parse_content_stream() via .operands/.operator instead of tuple-unpacking, since ContentStreamInstruction supports the legacy __getitem__-based iteration protocol but not __iter__, which mypy doesn't statically recognize. - pdfinfo/_worker.py: _pdf_pageinfo_concurrent's return type was Sequence[PageInfo | None] but always returns a real list; narrowed to match, fixing PdfInfo.pages' declared list[...] return type. --- src/ocrmypdf/_graft.py | 18 ++++++++++++++---- src/ocrmypdf/helpers.py | 31 +++++++++++++++++++++++++++++++ src/ocrmypdf/optimize.py | 15 +++++++++------ src/ocrmypdf/pdfinfo/_image.py | 16 ++++++++++------ src/ocrmypdf/pdfinfo/_worker.py | 4 ++-- src/ocrmypdf/pdfinfo/info.py | 29 +++++++++++++++++++++-------- 6 files changed, 87 insertions(+), 26 deletions(-) diff --git a/src/ocrmypdf/_graft.py b/src/ocrmypdf/_graft.py index 677f9682..6dd42019 100644 --- a/src/ocrmypdf/_graft.py +++ b/src/ocrmypdf/_graft.py @@ -6,11 +6,12 @@ from __future__ import annotations import logging +from collections.abc import Collection from contextlib import suppress from dataclasses import dataclass from enum import Enum from pathlib import Path -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, cast if TYPE_CHECKING: from ocrmypdf.hocrtransform import OcrElement @@ -18,6 +19,7 @@ if TYPE_CHECKING: from pikepdf import ( Dictionary, Name, + Object, Operator, Page, Pdf, @@ -181,9 +183,13 @@ def strip_invisible_text(pdf: Pdf, page: Page): render_mode_stack = [] text_objects = [] - for operands, operator in parse_content_stream(page, ''): + for instruction in parse_content_stream(page, ''): + operands, operator = instruction.operands, instruction.operator if operator == Operator('Tr'): - render_mode = operands[0] + # operands[0] is already a plain int under pikepdf's default + # (implicit) conversion mode, or a pikepdf.Object under explicit + # conversion mode; int() handles both. + render_mode = int(operands[0]) if operator == Operator('q'): render_mode_stack.append(render_mode) @@ -207,7 +213,11 @@ def strip_invisible_text(pdf: Pdf, page: Page): stream.extend(text_objects) text_objects.clear() - content_stream = unparse_content_stream(stream) + # pikepdf's Collection[...] parameter doesn't structurally match our + # _ObjectList-based tuples even though it works fine at runtime. + content_stream = unparse_content_stream( + cast('list[tuple[Collection[Object], Operator]]', stream) + ) page.Contents = Stream(pdf, content_stream) diff --git a/src/ocrmypdf/helpers.py b/src/ocrmypdf/helpers.py index 94cdc657..85a51a7d 100644 --- a/src/ocrmypdf/helpers.py +++ b/src/ocrmypdf/helpers.py @@ -337,6 +337,37 @@ def pikepdf_enable_mmap() -> None: log.debug("pikepdf mmap not available") +def pikepdf_get_int(obj: pikepdf.Object, key: pikepdf.Name, default: int = 0) -> int: + """Look up a key on a pikepdf dictionary/stream, returning a plain int. + + ``.get(key, default)``'s return type is the ambiguous ``Object | int``, + which does not support arithmetic or comparison against a plain int. In + pikepdf's default (implicit) conversion mode, a PDF Integer is already + unboxed to a native ``int`` by the time we see it here; under explicit + conversion mode it would instead be a ``pikepdf.Object``. ``int()`` + handles both, since ``Object`` implements ``__int__``. + """ + value = obj.get(key) + return int(value) if value is not None else default + + +def pikepdf_get_bool( + obj: pikepdf.Object, key: pikepdf.Name, default: bool = False +) -> bool: + """Look up a key on a pikepdf dictionary/stream, returning a plain bool. + + Unlike ``int()``/``float()``, ``bool()`` is not supported on + ``pikepdf.Object`` (it raises), so both conversion modes must be + handled explicitly. See :func:`pikepdf_get_int` for background. + """ + value = obj.get(key) + if value is None: + return default + if isinstance(value, bool): + return value + return value.as_bool(default) + + def running_in_docker() -> bool: """Returns True if we seem to be running in a Docker container.""" return Path('/.dockerenv').exists() diff --git a/src/ocrmypdf/optimize.py b/src/ocrmypdf/optimize.py index 6061fad2..52b4dde9 100644 --- a/src/ocrmypdf/optimize.py +++ b/src/ocrmypdf/optimize.py @@ -12,7 +12,7 @@ import threading from collections.abc import Callable, Iterator, MutableSet, Sequence from os import fspath from pathlib import Path -from typing import Any, NamedTuple, NewType +from typing import Any, NamedTuple, NewType, cast from zlib import compress import img2pdf @@ -37,7 +37,7 @@ from ocrmypdf._exec import ghostscript, jbig2enc, pngquant from ocrmypdf._jobcontext import PdfContext from ocrmypdf._progressbar import ProgressBar from ocrmypdf.exceptions import OutputFileAccessError -from ocrmypdf.helpers import IMG2PDF_KWARGS, safe_symlink +from ocrmypdf.helpers import IMG2PDF_KWARGS, pikepdf_get_int, safe_symlink log = logging.getLogger(__name__) @@ -527,8 +527,8 @@ def _find_deflatable_jpeg( ( # Don't flate very large images because it will slow down PDF viewers 1 <= options.optimize <= 2 - and image.get(Name.Width, 0) < FLATE_JPEG_THRESHOLD - and image.get(Name.Height, 0) < FLATE_JPEG_THRESHOLD + and pikepdf_get_int(image, Name.Width) < FLATE_JPEG_THRESHOLD + and pikepdf_get_int(image, Name.Height) < FLATE_JPEG_THRESHOLD ) or options.optimize == 3 ) @@ -608,10 +608,13 @@ def _transcode_png(pdf: Pdf, filename: Path, xref: Xref) -> bool: local_image = pdf.copy_foreign(foreign_image) im_obj = pdf.get_object(xref, 0) + # pikepdf's Object attribute access can't statically know Filter/ + # DecodeParms hold these specific subtypes, but a copied image's + # stream dictionary always does per the PDF spec. im_obj.write( local_image.read_raw_bytes(), - filter=local_image.Filter, - decode_parms=local_image.DecodeParms, + filter=cast('Name | Array | list[Name] | None', local_image.Filter), + decode_parms=cast('Dictionary | Array | None', local_image.DecodeParms), ) # Don't copy keys from the new image... diff --git a/src/ocrmypdf/pdfinfo/_image.py b/src/ocrmypdf/pdfinfo/_image.py index f7a34738..d64a7d84 100644 --- a/src/ocrmypdf/pdfinfo/_image.py +++ b/src/ocrmypdf/pdfinfo/_image.py @@ -7,6 +7,7 @@ from __future__ import annotations import logging from collections.abc import Iterator from decimal import Decimal +from typing import cast from pikepdf import ( Dictionary, @@ -20,7 +21,7 @@ from pikepdf import ( UnsupportedImageTypeError, ) -from ocrmypdf.helpers import Resolution +from ocrmypdf.helpers import Resolution, pikepdf_get_int from ocrmypdf.pdfinfo._contentstream import ( ContentsInfo, TextMarker, @@ -54,6 +55,7 @@ class ImageInfo: _comp: int | None _name: str + _enc: Encoding | None def __init__( self, @@ -90,8 +92,8 @@ class ImageInfo: # itself. Some PDF writers use this to create a grayscale stencil # mask. For our purposes, the effective size is the size of the # larger component (image or smask). - self._width = max(smask.get(Name.Width, 0), self._width) - self._height = max(smask.get(Name.Height, 0), self._height) + self._width = max(pikepdf_get_int(smask, Name.Width), self._width) + self._height = max(pikepdf_get_int(smask, Name.Height), self._height) if (mask := pim.obj.get(Name.Mask, None)) is not None and isinstance( mask, Stream | Dictionary ): @@ -99,8 +101,8 @@ class ImageInfo: # /Mask can be a Stream or an Array. If it's a Stream, # use its /Width and /Height if they are larger than the main # image's. - self._width = max(mask.get(Name.Width, 0), self._width) - self._height = max(mask.get(Name.Height, 0), self._height) + self._width = max(pikepdf_get_int(mask, Name.Width), self._width) + self._height = max(pikepdf_get_int(mask, Name.Height), self._height) # If /ImageMask is true, then this image is a stencil mask # (Images that draw with this stencil mask will have a reference to @@ -396,7 +398,9 @@ def _process_content_streams( # A Form XObject may provide its own matrix to map form space into # user space. Get this if one exists form_shorthand = container.get(Name.Matrix, Matrix()) - form_matrix = Matrix(form_shorthand) + # pikepdf's Matrix() stub omits the Object/Array overload, but the + # underlying C++ implementation accepts any 6-element numeric array. + form_matrix = Matrix(cast(Matrix, form_shorthand)) # Concatenate form matrix with CTM to ensure CTM is correct for # drawing this instance of the XObject diff --git a/src/ocrmypdf/pdfinfo/_worker.py b/src/ocrmypdf/pdfinfo/_worker.py index 01c4b458..95d088a6 100644 --- a/src/ocrmypdf/pdfinfo/_worker.py +++ b/src/ocrmypdf/pdfinfo/_worker.py @@ -6,7 +6,7 @@ from __future__ import annotations import atexit import logging -from collections.abc import Container, Sequence +from collections.abc import Container from contextlib import contextmanager from functools import partial from pathlib import Path @@ -84,7 +84,7 @@ def _pdf_pageinfo_concurrent( check_pages: Container[int], detailed_analysis: bool = False, miner_state: PdfMinerState | None = None, -) -> Sequence[PageInfo | None]: +) -> list[PageInfo | None]: pages: list[PageInfo | None] = [None] * len(pdf.pages) def update_pageinfo(page: PageInfo, pbar: ProgressBar): diff --git a/src/ocrmypdf/pdfinfo/info.py b/src/ocrmypdf/pdfinfo/info.py index 5001bd79..f704d09e 100644 --- a/src/ocrmypdf/pdfinfo/info.py +++ b/src/ocrmypdf/pdfinfo/info.py @@ -16,12 +16,12 @@ from pathlib import Path from typing import NamedTuple from pdfminer.layout import LTPage, LTTextBox -from pikepdf import Name, Page, Pdf +from pikepdf import Name, Object, Page, Pdf from ocrmypdf._concurrent import Executor, SerialExecutor from ocrmypdf._pageboxes import coerce_box from ocrmypdf.exceptions import EncryptedPdfError -from ocrmypdf.helpers import Resolution +from ocrmypdf.helpers import Resolution, pikepdf_get_bool, pikepdf_get_int from ocrmypdf.pdfinfo._contentstream import TextboxInfo, TextMarker, VectorMarker from ocrmypdf.pdfinfo._image import ImageInfo, _process_content_streams from ocrmypdf.pdfinfo._types import FloatRect @@ -160,6 +160,10 @@ class PageInfo: check_this_page = pageno in check_pages if check_this_page and detailed_analysis: + # miner_state is only None when detailed_analysis is False (see + # PdfInfo.__init__, which ties the two together), so it must be + # set here. + assert miner_state is not None page_analysis = miner_state.get_page_analysis(pageno) if page_analysis is not None: self._textboxes = list( @@ -175,7 +179,11 @@ class PageInfo: self._has_text = None # i.e. "no information" userunit = page.get(Name.UserUnit, Decimal(1.0)) - if not isinstance(userunit, Decimal): + if isinstance(userunit, Object): + # Only reachable under pikepdf's explicit conversion mode; the + # default (implicit) mode already unboxes to int/float/Decimal. + userunit = Decimal(userunit.as_float()) + elif not isinstance(userunit, Decimal): userunit = Decimal(userunit) self._userunit = userunit self._width_inches = width_pt * userunit / Decimal(72.0) @@ -189,7 +197,7 @@ class PageInfo: self._has_text = False self._images = [] for info in _process_content_streams( - pdf=pdf, container=page, shorthand=userunit_shorthand + pdf=pdf, container=page.obj, shorthand=userunit_shorthand ): if isinstance(info, VectorMarker): self._has_vector = True @@ -446,16 +454,21 @@ class PdfInfo: detailed_analysis=detailed_analysis, miner_state=miner_state, ) - self._needs_rendering = pdf.Root.get(Name.NeedsRendering, False) + self._needs_rendering = pikepdf_get_bool(pdf.Root, Name.NeedsRendering) if Name.AcroForm in pdf.Root: if ( len(pdf.Root.AcroForm.get(Name.Fields, [])) > 0 or Name.XFA in pdf.Root.AcroForm ): self._has_acroform = True - self._has_signature = bool(pdf.Root.AcroForm.get(Name.SigFlags, 0) & 1) - self._is_tagged = bool( - pdf.Root.get(Name.MarkInfo, {}).get(Name.Marked, False) + self._has_signature = bool( + pikepdf_get_int(pdf.Root.AcroForm, Name.SigFlags) & 1 + ) + mark_info = pdf.Root.get(Name.MarkInfo) + self._is_tagged = ( + pikepdf_get_bool(mark_info, Name.Marked) + if mark_info is not None + else False ) self._has_structure_tree = Name.StructTreeRoot in pdf.Root