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

DOC: merge_transformed_page() #1647

Merged
merged 3 commits into from
Feb 25, 2023
Merged

DOC: merge_transformed_page() #1647

merged 3 commits into from
Feb 25, 2023

Conversation

paternal
Copy link
Contributor

There is no example for merge_transformed_page(), although several issues could have been avoided if such an example existed (#1630, #1426, and issues that users do not bother to open). This pull request adds such an example.

Thanks for your work!

-- Louis

@codecov
Copy link

codecov bot commented Feb 20, 2023

Codecov Report

Base: 91.92% // Head: 91.92% // No change to project coverage 👍

Coverage data is based on head (7d6cb4f) compared to base (4e276b2).
Patch has no changes to coverable lines.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1647   +/-   ##
=======================================
  Coverage   91.92%   91.92%           
=======================================
  Files          33       33           
  Lines        6375     6375           
  Branches     1272     1272           
=======================================
  Hits         5860     5860           
  Misses        327      327           
  Partials      188      188           

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
Collaborator

pubpub-zz commented Feb 20, 2023

@paternal
Thanks for you contribution. I would more likely recommend to use .merge_transformed_page() instead of doing the transform before merging. The advantage is that you can reuse the page.
we could keep the merge_page for the watermarking only.
Also maybe you could propose to update the example for underlay watermark using the over=False new option.

@paternal
Copy link
Contributor Author

@paternal Thanks for you contribution. I would more likely recommend to use .merge_transformed_page() instead of doing the transform before merging.

Unless I misunderstood you, it is already done that way: since most of the examples on this page use merge_page(), I started with a naïve solution using this, that does not work. I then explain why it does not work, and I go on with the right solution using merge_transformed_page().

The advantage is that you can reuse the page. we could keep the merge_page for the watermarking only. Also maybe you could propose to update the example for underlay watermark using the over=False new option.

This page? https://pypdf.readthedocs.io/en/stable/user/add-watermark.html?highlight=watermark
I can update it.

@pubpub-zz
Copy link
Collaborator

@paternal Thanks for you contribution. I would more likely recommend to use .merge_transformed_page() instead of doing the transform before merging.

Unless I misunderstood you, it is already done that way: since most of the examples on this page use merge_page(), I started with a naïve solution using this, that does not work. I then explain why it does not work, and I go on with the right solution using merge_transformed_page().

I did not read correctly your proposal with the diff mod of github 🤭
I've seen so many people using transformation, that is currently not applying any changes on the annots.

The advantage is that you can reuse the page. we could keep the merge_page for the watermarking only. Also maybe you could propose to update the example for underlay watermark using the over=False new option.

This page? https://pypdf.readthedocs.io/en/stable/user/add-watermark.html?highlight=watermark I can update it.

yes🙂

@paternal
Copy link
Contributor Author

Done.

English is not my first language: feel free to rephrase everything.


# Transforming several copies of the same page

We have designed the following business card (A8 format) to advertize our new startup.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hahaha, I love it 😄 ❤️

@MartinThoma MartinThoma changed the title Document merge_transformed_page() DOC: merge_transformed_page() Feb 25, 2023
@MartinThoma MartinThoma added nf-documentation Non-functional change: Documentation workflow-merge From a users perspective, merging is the affected feature/workflow labels Feb 25, 2023
@MartinThoma MartinThoma merged commit cc32b59 into py-pdf:main Feb 25, 2023
@MartinThoma
Copy link
Member

@paternal Thank you 🙏

If you want, I can add you to the contributors page: https://pypdf.readthedocs.io/en/latest/meta/CONTRIBUTORS.html
Just let me know which name I should use :-) (just Louis? paternal?)

@paternal
Copy link
Contributor Author

paternal commented Feb 25, 2023

Just let me know which name I should use :-) (just Louis? paternal?)

Louis Paternault https://framagit.org/spalax

Thanks

@MartinThoma
Copy link
Member

done :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
nf-documentation Non-functional change: Documentation 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.

3 participants