-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
MAINT: indirect_ref/ido ➔ indirect_reference, dest➔ page_destination #1467
Conversation
…dest_ref in pdfwriter.add_outline_item_destination and pdfwriter.add_named_destination_object
…object_from_stream and pdfwriter.get_object
…stead of user_pwd and owner_pwd
… reflect changes to pdfWriter class
Codecov ReportBase: 93.85% // Head: 93.76% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #1467 +/- ##
==========================================
- Coverage 93.85% 93.76% -0.09%
==========================================
Files 30 30
Lines 5464 5470 +6
Branches 1046 1048 +2
==========================================
+ Hits 5128 5129 +1
- Misses 202 206 +4
- Partials 134 135 +1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
@kygoben Thank you for approaching this! When we make those changes, we want to make them first in a non-breaking way. That means people who use the method with keyword parameters should receive a warning (but not an exception) that their usage is deprecated. We also need to add this to the migration guide: https://pypdf2.readthedocs.io/en/latest/user/migration-1-to-2.html Finally, we should typically use the long form. That means:
See #1365 for a similar PR |
Hi @MartinThoma, I am a bit unsure how to completely approach your comments. For instance, you said that the migration guide would need to be updated. Where can I find this, and how should this be updated? And the second one is to give people a warning when using the keywords. Thank you for your help :) |
Not sure what I did incorrectly in these last modifications. Could someone give me some pointers to fix the test outcomes? |
I'll have a look at the weekend. I was at a company event this week and moving/getting married last week :-) |
If you look into |
It would also be good to add a property |
I'll add this either before the PyPDF2==3.0.0 release or earlier. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rename Page.indirect_reference
temporarily back to Page.indirect_ref
to fix the PR
This change should allow me to merge this PR.
Documentation (DOC) - Deduplicate extract_text docstring (#1485) - How to cite PyPDF2 (#1476) Maintenance (MAINT) Consistency changes: - indirect_ref/ido ➔ indirect_reference, dest➔ page_destination (#1467) - owner_pwd/user_pwd ➔ owner_password/user_password (#1483) - position ➜ page_number in Merger.merge (#1482) - indirect_ref ➜ indirect_reference (#1484) [Full Changelog](2.12.0...2.12.1)
From issue #1187 used the descriptions from @mtd91429 to make a few modifications to PdfWriter and PdfReader
indirect_ref
➔indirect_reference
andido
➔indirect_reference
inPdfReader._get_object_from_stream
andPdfWriter.get_object
dest
➔page_destination
anddest_ref
➔page_destination_ref
inPdfWriter.add_outline_item_destination
andPdfWriter.add_named_destination_object