-
Notifications
You must be signed in to change notification settings - Fork 1.5k
BUG: PdfWriter.add_uri was setting the wrong type #2406
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
/TYPE for a link should be "/Annot" not "/Annots" This change makes add_uri work; previously, it did not.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2406 +/- ##
=======================================
Coverage 94.42% 94.42%
=======================================
Files 49 49
Lines 7961 7961
Branches 1608 1608
=======================================
Hits 7517 7517
Misses 274 274
Partials 170 170 ☔ View full report in Codecov by Sentry. |
Thanks. Do we have a chance to add a corresponding test as well? |
add_uri is already tested in test_writer.py. That test requires a
manual step to check that the generated PDF has links that work; that
step would be sufficient my patch. I don't know how to formalize that
manual step.
My casual test is to generate a blank page with a full-page link to
cnn.com:
writer = pypdf.PdfWriter()
writer.add_blank_page(width=500, height=500)
writer.add_uri(0, 'https://cnn.com', [0, 0, 499, 499])
writer.write('demo.pdf')
|
However, Good catch! 👍 |
@pmiller66 Thank you for your PR 🙏 It will be part of pypdf==4.0.0. As this is a major release, it might take a bit longer than usual for the release on PyPI. I want to get some breaking changes into it. My guess is that we will publish pypdf==4.0.0 end of January/early February |
If you want, I can add you to https://pypdf.readthedocs.io/en/latest/meta/CONTRIBUTORS.html :-) |
Regarding the testing: I don't see an easy way to meaningfully test this. Essentially, we would either (1) need to rely on another library and make a comparison between the generated PDFs or (2) have some RPA-like process and thus relying on a PDF viewer + the RPA software. Nothing you need to do in this PR as it's quite obvious that this change is correct and the PR improves pypdf :-) |
Sure, honored. --P
Phil Miller
11 Coventry Close
Avondale, GA 30002
***@***.***
404.296.1221
…------ Original Message ------
From "Martin Thoma" ***@***.***>
To "py-pdf/pypdf" ***@***.***>
Cc "pmiller66" ***@***.***>; "Mention"
***@***.***>
Date 1/17/2024 4:48:48 PM
Subject Re: [py-pdf/pypdf] BUG: PdfWriter.add_uri was setting the wrong
type (PR #2406)
If you want, I can add you to
https://pypdf.readthedocs.io/en/latest/meta/CONTRIBUTORS.html :-)
—
Reply to this email directly, view it on GitHub
<#2406 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AA72UFCOH4JOXNDFLZAQN7LYPBBMBAVCNFSM6AAAAABBYMPZH2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOJWHEZTOOBQGI>.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
## What's new pypdf==4.0.0 is a big milestone forward: * We finally have a layout-mode text extraction. This enables users who want to detect / extract tables with heuristics to give it a try. * We deprecated a lot of the old PyPDF2 API that was either not following PEP8 naming styles or was not using a property. Users comming from PyPDF2 might want to switch first to pypdf<4.0.0 to get helpful error messages that show the new API in their speicific cases. A big 'Thank you!' the the whole pypdf community for your work. Thanks to you, pypdf is better than ever. Kudos to @shartzog who added the layout-mode with his first contribution! ### Deprecations (DEP) - Drop Python 3.6 support (#2369) by @MartinThoma - Remove deprecated code (#2367) by @MartinThoma - Remove deprecated XMP properties (#2386) by @stefan6419846 ### New Features (ENH) - Add "layout" mode for text extraction (#2388) by @shartzog - Add Jupyter Notebook integration for PdfReader (#2375) by @MartinThoma - Improve/rewrite PDF permission retrieval (#2400) by @stefan6419846 ### Bug Fixes (BUG) - PdfWriter.add_uri was setting the wrong type (#2406) by @pmiller66 - Add support for GBK2K cmaps (#2385) by @stefan6419846 ### Documentation (DOC) - Add pmiller66 for #2406 as a contributor by @MartinThoma - Add missing expand parameter (#2393) by @Atomnp - Resolve build warnings (#2380) by @stefan6419846 - Fix testing prerequisites (#2381) by @stefan6419846 - Improve formatting of contributors page (#2383) by @stefan6419846 - Add Tobeabellwether as a contributor for #2341 by @MartinThoma ### Developer Experience (DEV) - Make dependabot aware of our PR prefixes (#2415) by @stefan6419846 - Fail on Sphinx issues (#2405) by @stefan6419846 - Move title check to own workflow (#2384) by @MasterOdin - Write to temporary files instead of the working directory (#2379) by @stefan6419846 - Ensure that the PR titles have the correct format (#2378) by @stefan6419846 ### Maintenance (MAINT) - Complete FileSpecificationDictionaryEntries constants (#2416) by @MartinThoma - Return None instead of -1 when page is not attached (#2376) by @MartinThoma - Replace warning with logging.error (#2377) by @MartinThoma ### Testing (TST) - Add missing pytest.mark.samples annotations (#2412) by @kitterma - Correctly close temporary files (#2396) by @stefan6419846 - Fix side effect #2379 (#2395) by @pubpub-zz - Add test for layout extraction mode (#2390) by @MartinThoma ### Code Style (STY) - Use the UserAccessPermissions enum (#2398) by @MartinThoma - Run black (#2370) by @MartinThoma [Full Changelog](3.17.4...4.0.0)
BUG: /TYPE for a link should be "/Annot" not "/Annots" This change makes add_uri work; previously, it did not.