-
Notifications
You must be signed in to change notification settings - Fork 1.5k
DOC: Add outlines documentation and link it in User Guide #3511
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
stefan6419846
left a comment
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.
Thanks for the PR.
- Please have a look at the failing tests.
- Please move user docs to the user-specific directory.
|
Hi @stefan6419846 , I have resolved the failing tests and moved the user docs to the user-specific directory. Please review. |
stefan6419846
left a comment
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.
Could you please align this new guide with the other ones which have changed to execute the actual example code in the meantime?
|
Hi @stefan6419846, Thanks for the feedback, I’ll align the new guide with the other ones that execute the actual example code and update the PR soon. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3511 +/- ##
==========================================
+ Coverage 97.16% 97.31% +0.15%
==========================================
Files 57 55 -2
Lines 9809 9769 -40
Branches 1781 1780 -1
==========================================
- Hits 9531 9507 -24
+ Misses 167 155 -12
+ Partials 111 107 -4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
a7a0dcf to
29301f5
Compare
29301f5 to
fa6c927
Compare
|
I have modified the PR to execute the actual code examples. Please review. |
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.
Thanks for the update. Judging from the structure etc., it seems like this is mostly auto-generated using AI - which is no issue as long as you have checked the changes yourself and it follows our usual structure.
Unfortunately, I cannot merge this as-is:
- Could we please use subsections instead of separate files for reading and writing?
- Why is the introduction a note in one case, but regular text in the other. I guess we should make this regular text.
- The user docs are not for resembling the exact API and describing the parameters. These should go into the docstrings. If required, link to the corresponding methods/classes in your description using the corresponding Sphinx directive/role.
- Please do not save arbitrary (and large) files in the
resourcesdirectory. If you keep the order "Add" - "Read", you should be able to re-use the generated file in the second examples. - Do not import from internal modules like
_fit. Use the public API only! - Instead of creating a reader and writer and adding the pages manually, just use
writer = PdfWriter(clone_from="input.pdf"). - The description of what the code does usually goes before it. This should be no list, but a short descriptive text.
- If you decide to include screenshots, please make sure that they ideally resemble the input document and you have the necessary rights to include it. If highlighting is required, use proper drawing tools to get nice rectangles instead of arbitrary drawings.
Please note that I might have missed some aspects during this review due to merging this requires some larger changes anyway.
|
Hi @stefan6419846, I have incorporated the changes you suggested. requesting for review again. Thank you |
resources/output.pdf
Outdated
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.
- This file is rather large. Can we use a smaller one - or even one of the existing?
- Do you own all the necessary copyrights to actually distribute this?
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.
I have reduced the pdf file size and I have all necessary copyrights because I had createad it by myself using google docs.
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.
I am still not fully convinced why we cannot use one of the existing files? Especially since input.pdf is rather generic and we try to keep the changes to the resources directory as small as possible.
resources/output.pdf
Outdated
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.
I am still not fully convinced why we cannot use one of the existing files? Especially since input.pdf is rather generic and we try to keep the changes to the resources directory as small as possible.
|
Hi @stefan6419846, Thanks for the feedback. we need to upload Please review. Thank you |
The process in the docs adds the outlines, thus reading the output from the previous step in the next step should still work, without having to add an |
9b21a1b to
467f8c7
Compare
Hi @stefan6419846, However, to make the reading section work properly without overwriting, I'm saving each write code example section namely simple, nested and advanced in separate files namely Please review, Thank you. |
stefan6419846
left a comment
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.
Thanks for your patience.
## What's new ### Security (SEC) - Improve handling of partially broken PDF files (#3594) by @stefan6419846 ### Deprecations (DEP) - Block common page content modifications when assigned to reader (#3582) by @stefan6419846 ### New Features (ENH) - Embellishments to generated text appearance streams (#3571) by @PJBrs ### Bug Fixes (BUG) - Do not consider multi-byte BOM-like sequences as BOMs (#3589) by @stefan6419846 ### Robustness (ROB) - Avoid empty FlateDecode outputs without warning (#3579) by @stefan6419846 ### Documentation (DOC) - Add outlines documentation and link it in User Guide (#3511) by @mainuddin-md ### Developer Experience (DEV) - Add PyPy 3.11 to test matrix and benchmarks (#3574) by @rassie ### Maintenance (MAINT) - Fix compatibility with Pillow >= 12.1.0 (#3590) by @stefan6419846 [Full Changelog](6.5.0...6.6.0)
This adds documentation for reading and adding PDF outlines/bookmarks
as requested in issue #3484. Linked in User Guide.