Skip to content

BUG: Switch from trimbox to cropbox when merging pages#1622

Merged
MartinThoma merged 2 commits intomainfrom
configurable-cropbox
Feb 11, 2023
Merged

BUG: Switch from trimbox to cropbox when merging pages#1622
MartinThoma merged 2 commits intomainfrom
configurable-cropbox

Conversation

@MartinThoma
Copy link
Copy Markdown
Member

@MartinThoma MartinThoma commented Feb 10, 2023

@pubpub-zz Continuing our discussion from #879 and #1426 :-)

I'm not sure if exposing this that prominently is a good idea or if we should simply change it... but allowing people to easily switch back would potentially enable people to still get the old behavior if they need to

I also don't know if MERGE_CROP_BOX is a good name for this attribute.

@codecov
Copy link
Copy Markdown

codecov bot commented Feb 10, 2023

Codecov Report

Base: 91.92% // Head: 91.92% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (42a61ce) compared to base (f5ac79b).
Patch coverage: 100.00% of modified lines in pull request are covered.

❗ Current head 42a61ce differs from pull request most recent head 28e96cd. Consider uploading reports for the commit 28e96cd to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1622   +/-   ##
=======================================
  Coverage   91.92%   91.92%           
=======================================
  Files          33       33           
  Lines        6374     6375    +1     
  Branches     1272     1272           
=======================================
+ Hits         5859     5860    +1     
  Misses        327      327           
  Partials      188      188           
Impacted Files Coverage Δ
pypdf/_page.py 90.54% <100.00%> (+0.01%) ⬆️

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.
📢 Do you have feedback about the report comment? Let us know in this issue.

@pubpub-zz
Copy link
Copy Markdown
Collaborator

pubpub-zz commented Feb 11, 2023

I like your idea. 👏👏👏
about the name It does not sound too bad. just a few line at the bottom of https://pypdf.readthedocs.io/en/latest/user/cropping-and-transforming.html should be sufficient

PS : I would be interested in a quick merge in order to upgrade the remaining merges identified in #1615

@MartinThoma MartinThoma changed the title ENH: Make used cropbox configurable BUG: Switch from trimbox to cropbox when merging pages Feb 11, 2023
@MartinThoma MartinThoma merged commit c70dcd0 into main Feb 11, 2023
@MartinThoma MartinThoma deleted the configurable-cropbox branch February 11, 2023 13:34
@MartinThoma
Copy link
Copy Markdown
Member Author

@pubpub-zz Merged :-) Thanks for pointing out that MERGE_CROP_BOX should be in the docs 👍 And thanks for bringing up this topic multiple times 😄

@MartinThoma MartinThoma added workflow-merge From a users perspective, merging is the affected feature/workflow is-bug From a users perspective, this is a bug - a violation of the expected behavior with a compliant PDF labels Feb 11, 2023
MartinThoma added a commit that referenced this pull request Feb 12, 2023
Bug Fixes (BUG):
-  Switch from trimbox to cropbox when merging pages (#1622)
-  Text extraction not working with one glyph to char sequence (#1620)

Robustness (ROB):
-  Fix 2 cases of "object has no attribute \'indirect_reference\'" (#1616)

Testing (TST):
-  Add multiple retry on get_url for external PDF downloads (#1626)

[Full Changelog](3.4.0...3.4.1)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

is-bug From a users perspective, this is a bug - a violation of the expected behavior with a compliant PDF workflow-merge From a users perspective, merging is the affected feature/workflow

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants