Skip to content

BUG: Type _id_translated to match what it holds - #3970

Merged
stefan6419846 merged 2 commits into
py-pdf:mainfrom
RavSinghChandan:fix-id-translated-type
Aug 19, 2026
Merged

BUG: Type _id_translated to match what it holds#3970
stefan6419846 merged 2 commits into
py-pdf:mainfrom
RavSinghChandan:fix-id-translated-type

Conversation

@RavSinghChandan

Copy link
Copy Markdown
Contributor

_id_translated is declared dict[int, dict[int, int]], but each inner
mapping also stores the source document under a "PreventGC" key so it is
not garbage collected while the translation table is live. The two writes
needed # type: ignore[index] to get past that, and a runtime protocol
check on PdfWriterProtocol fails on the string key.

Widened to dict[int, dict[Union[int, str], Any]], which is what the
dictionary actually contains, and dropped the two ignores that became
unnecessary. Every reader looks entries up by idnum rather than iterating,
so behaviour is unchanged.

Under make testtype this accounts for 55 failures across test_generic and
test_writer; they go to 0. mypy reports the same 11 pre-existing errors
before and after, none in these files.

`_id_translated` is declared `dict[int, dict[int, int]]`, but each inner
mapping also stores the source document under a `"PreventGC"` key so it
is not garbage collected while the translation table is live. The two
writes needed `# type: ignore[index]` to get past that, and a runtime
protocol check on `PdfWriterProtocol` fails on the string key.

Widen the declaration to `dict[int, dict[Union[int, str], Any]]`, which
is what the dictionary actually contains, and drop the two ignores that
are then unnecessary. Every reader looks entries up by idnum rather than
iterating, so behaviour is unchanged.

Under `make testtype` this accounts for 55 failures across test_generic
and test_writer; they go to 0. mypy reports the same 11 pre-existing
errors before and after, none in these files.
@codecov

codecov Bot commented Aug 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.94%. Comparing base (1bce7a7) to head (84b0cc9).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3970   +/-   ##
=======================================
  Coverage   97.94%   97.94%           
=======================================
  Files          57       57           
  Lines       11031    11031           
  Branches     2065     2065           
=======================================
  Hits        10804    10804           
  Misses        126      126           
  Partials      101      101           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread pypdf/_protocols.py Outdated
It is the only string key the mapping ever holds, so the narrower type
is the accurate one.
@RavSinghChandan

Copy link
Copy Markdown
Contributor Author

Fair point - "PreventGC" is the only string key that ever goes in there, so
Literal is the accurate type. Switched it. Also trimmed the comment down while
I was in there.

@stefan6419846
stefan6419846 merged commit 1856f9e into py-pdf:main Aug 19, 2026
33 of 34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants