Skip to content
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

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

Merged
merged 2 commits into from
Aug 2, 2023
Merged

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

merged 2 commits into from
Aug 2, 2023

Conversation

mnmtz
Copy link
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
Collaborator

Could you please add a corresponding test case as well?

@mnmtz
Copy link
Contributor Author

mnmtz commented Aug 1, 2023

example_129.pdf

@pubpub-zz
Copy link
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

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
14 checks passed
@MartinThoma
Copy link
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
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