Skip to content

Replace pypdf2 1.28.6 with pypdf 6.20.1 - #876

Merged
rtibbles merged 1 commit into
learningequality:mainfrom
rtibblesbot:issue-872-917443
Oct 22, 2026
Merged

rtibbles merged 1 commit into
learningequality:mainfrom
rtibblesbot:issue-872-917443

Conversation

@rtibblesbot

@rtibblesbot rtibblesbot commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • PyPDF2 is deprecated; its 3.x line drops the API PDFParser relies on.
  • Moves PDFParser and its tests to pypdf, opened with strict=True plus reader hooks keeping PyPDF2 1.26's lenient handling of malformed PDFs.

References

Fixes #872. Closed bump: #668.

Reviewer guidance

Suggested review: Full code review.

  • Decide whether the exact pypdf==6.20.1 pin, needed because CustomPDFReader overrides six private pypdf methods, is acceptable given that it blocks pypdf security releases until someone re-pins by hand.
  • Confirm that strict=True pypdf behaves like 1.28.6 on real chef PDFs: the tests and the samples match base, but other strict-only raises (e.g. a GoTo outline action missing /S or /D) still fail get_toc.
  • Confirm that removing the module-level CustomPDFWriter/CustomDestination names, and making encrypted PDFs fail in open instead of get_toc, is fine for external chef scripts.

tests/test_pdfutils.py covers the change on CI.

AI usage

Used Claude Code to port PDFParser and its tests to pypdf and to recheck each PyPDF2 override. Verified with the full test suite and prek.


Written by rtibblesbot, an LLM-based coding agent.

@rtibblesbot

🟢 Updating PR

Last updated: 2026-10-22 22:38 UTC

- Port PDFParser and tests to the PdfReader/PdfWriter API
- Open with strict=True; keep reader hooks for 1.26 leniency
- Drop CustomPDFWriter and CustomDestination

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@rtibblesbot
rtibblesbot marked this pull request as ready for review October 22, 2026 09:04
@rtibblesbot
rtibblesbot requested review from a team and leq-bot October 22, 2026 09:04

@rtibbles rtibbles left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test coverage is sufficient, and existing tests still pass.

@rtibbles
rtibbles merged commit 029294b into learningequality:main Oct 22, 2026
37 checks passed
@rtibblesbot

Copy link
Copy Markdown
Contributor Author

Quick check on rtibblesbot's work on this PR: react 👍 helpful · 😕 mixed · 👎 not helpful. Reply with anything specific.

@rtibblesbot
rtibblesbot deleted the issue-872-917443 branch October 22, 2026 22:41
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.

Migrate ricecooker/utils/pdf.py from PyPDF2 1.28.6 to pypdf

2 participants