Skip to content

BUG: Added check for field /Info when cloning reader document#2055

Merged
MartinThoma merged 2 commits intopy-pdf:mainfrom
mnmtz:main
Aug 2, 2023
Merged

BUG: Added check for field /Info when cloning reader document#2055
MartinThoma merged 2 commits intopy-pdf:mainfrom
mnmtz:main

Conversation

@mnmtz
Copy link
Copy Markdown
Contributor

@mnmtz mnmtz commented Aug 1, 2023

Added check for optional field /Info in document root when cloning reader document. (Fixes issue #2049)

Added check for optional field /Info in document root when cloning reader document. (Fixes issue #2049)
@mnmtz mnmtz changed the title Update _writer.py BUG: Added check for field /Info when cloning reader document (#2049) Aug 1, 2023
@mnmtz mnmtz changed the title BUG: Added check for field /Info when cloning reader document (#2049) BUG: Added check for field /Info when cloning reader document Aug 1, 2023
@mnmtz mnmtz changed the title BUG: Added check for field /Info when cloning reader document ENH: Added check for field /Info when cloning reader document Aug 1, 2023
@stefan6419846
Copy link
Copy Markdown
Collaborator

Could you please add a corresponding test case as well?

@mnmtz
Copy link
Copy Markdown
Contributor Author

mnmtz commented Aug 1, 2023

example_129.pdf

@pubpub-zz
Copy link
Copy Markdown
Collaborator

@mnmtz
What @stefan6419846 means, is can you add a test in test_reader.py:
you should be able to create a new test function which will create a new pdfwriter with the parameter clone_from receiving the bytes from the url you've posted above.

@codecov
Copy link
Copy Markdown

codecov bot commented Aug 1, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (534c7b4) 94.17% compared to head (94e296d) 94.17%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2055   +/-   ##
=======================================
  Coverage   94.17%   94.17%           
=======================================
  Files          41       41           
  Lines        7332     7333    +1     
  Branches     1441     1442    +1     
=======================================
+ Hits         6905     6906    +1     
  Misses        266      266           
  Partials      161      161           
Files Changed Coverage Δ
pypdf/_writer.py 87.81% <100.00%> (+0.01%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@MartinThoma MartinThoma changed the title ENH: Added check for field /Info when cloning reader document BUG: Added check for field /Info when cloning reader document Aug 2, 2023
@MartinThoma MartinThoma merged commit 34b58c8 into py-pdf:main Aug 2, 2023
@MartinThoma
Copy link
Copy Markdown
Member

Thank you for your contribution @mnmtz :-) If you want, I can add you to https://pypdf.readthedocs.io/en/latest/meta/CONTRIBUTORS.html :-)

@MartinThoma
Copy link
Copy Markdown
Member

I've labeled this with "BUG", because from the users perspective it fixed something that was wrong. This is mainly relevant for where to put it in the CHANGELOG. I doubt any user would get excited about this change, although it's an important improvement. ENH (enhancements, new features) are typically things that were not supported beforehand and expand the capabilities of pypdf.

There are many edge cases. For example, @pubpub-zz recently improved the image support. There I decided to go with ENH, because it were bigger changes

MartinThoma added a commit that referenced this pull request Aug 6, 2023
### New Features (ENH)
-  Add `level` parameter to compress_content_streams (#2044)
-  Process /uniHHHH for text_extract (#2043)

### Bug Fixes (BUG)
-  Fix AnnotationBuilder.link (#2066)
-  JPX image without ColorSpace  (#2062)
-  Added check for field /Info when cloning reader document (#2055)
-  Fix indexed/CMYK images (#2039)

### Maintenance (MAINT)
-  Cryptography as primary dependency (#2053)

[Full Changelog](3.14.0...3.15.0)
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.

4 participants