STY: Tweak PdfWriter #3337
STY: Tweak PdfWriter #3337
Conversation
Small changes including making method insert_blank_page more flexible.
Small changes including making method insert_blank_page more flexible.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3337 +/- ##
=======================================
Coverage ? 96.73%
=======================================
Files ? 53
Lines ? 9062
Branches ? 1678
=======================================
Hits ? 8766
Misses ? 177
Partials ? 119 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Could you please have a look at the coverage? All (new) conditions should be covered be appropriate tests. |
stefan6419846
added
the
needs-test
| @@ -672,10 +671,12 @@ def insert_blank_page( | |||
| and previous page does not exist. | |||
|
|
|||
| """ | |||
| if width is None or (height is None and index < self.get_num_pages()): | |||
| if (width is None or height is None) and index < self.get_num_pages(): | |||
There was a problem hiding this comment.
Doesn't this change when this is executed?
There was a problem hiding this comment.
Functionality is changed. If width is None we only want to branch if index < self.get_num_pages().
There was a problem hiding this comment.
Could you please elaborate why? Has this been a bug we need to fix or are there other reasons?
There was a problem hiding this comment.
width and height are kinda symmetric, I do not think they should be handled differently. It is slightly confusing they are handled differently. So this changes this, and also stops branching when width is None and index >= self.get_num_pages().
Co-authored-by: Stefan <96178532+stefan6419846@users.noreply.github.com>
Small changes including making method insert_blank_page more flexible.
Small changes including making method insert_blank_page more flexible.
Small changes including making method insert_blank_page more flexible.
| @@ -236,6 +236,10 @@ def writer_operate(writer: PdfWriter) -> None: | |||
| writer.page_mode = NameObject("/UseOC") | |||
| assert writer._get_page_mode() == "/UseOC" | |||
| writer.insert_blank_page(width=100, height=100) | |||
| page = writer.insert_blank_page(width=100) | |||
| assert page.mediabox.height == 100 | |||
There was a problem hiding this comment.
Please use a dedicated test method for this and add assertions for the width and height respectively as well to ensure that it is indeed working correctly.
Small changes including making method insert_blank_page more flexible.