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

ENH: Add --layout parameter to up2 #67

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

mulla028
Copy link

@mulla028 mulla028 commented Oct 30, 2024

Closes #64

Overview

This PR introduces an optional --layout parameter to the up2 command, allowing users to arrange PDF pages in configurable grid layouts. Supported layouts include:

  • 2x2: 2 columns, 2 rows per page
  • 3x3: 3 columns, 3 rows per page
  • 1x2: 1 column, 2 rows per page
  • 2x1: 2 columns, 1 row per page

If --layout is not specified, the command defaults to the original behavior, ensuring full backward compatibility.

Changes

  1. up2.py:

    • Added a new up2_main function to manage both default behavior and specified layouts.
    • Updated page dimension access to mediabox for compatibility with recent versions of pypdf.
    • Retained the original main function for backward compatibility, though future calls should use up2_main.
  2. cli.py:

    • Modified the up2 command to call up2_main with an optional --layout parameter for layout configurations.
    • Retained all original commands and functionality to ensure no changes to existing CLI behavior.

Testing

To test each layout configuration, run the following commands:

# Default behavior (no layout specified)
python cli.py up2 path/to/input.pdf path/to/output_default.pdf

# 2x2 layout
python cli.py up2 path/to/input.pdf path/to/output_2x2.pdf --layout 2x2

# 3x3 layout
python cli.py up2 path/to/input.pdf path/to/output_3x3.pdf --layout 3x3

# 1x2 layout
python cli.py up2 path/to/input.pdf path/to/output_1x2.pdf --layout 1x2

# 2x1 layout
python cli.py up2 path/to/input.pdf path/to/output_2x1.pdf --layout 2x1

… 1x2, 2x1) without impacting default behavior.
…gurations, ensuring basic functionality and output verification.
@mulla028
Copy link
Author

@Lucas-C
I have written yesterday a test for --layout feature, but wasn't sure if you need it. According to your recent feedback in Issue #63, I found that you would like to see some tests. Eventually, I just pushed it 😆

@pastor-robert pastor-robert mentioned this pull request Dec 2, 2024
@MartinThoma MartinThoma changed the title Add Flexible PDF Layout Options with --layout Parameter ENH: Add Flexible PDF Layout Options with --layout Parameter Dec 8, 2024
Comment on lines +15 to +36
# File paths for the input and output PDFs
input_pdf = Path("test_input.pdf")
output_pdf = Path("test_output.pdf")

# Create the test PDF
create_test_pdf(input_pdf)

# List of layouts to test
layouts = ["2x2", "3x3", "1x2", "2x1"]
for layout in layouts:
print(f"\nTesting layout: {layout}")
up2_main(input_pdf, output_pdf, layout=layout)

# Read the output PDF and print the number of pages
reader = PdfReader(str(output_pdf))
print(f"Output PDF for layout {layout} has {len(reader.pages)} pages.")

# Clean up
if output_pdf.exists():
os.remove(output_pdf)
if input_pdf.exists():
os.remove(input_pdf)
Copy link
Member

Choose a reason for hiding this comment

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

We use pytest as a test suite to automatically run the tests and check if they succeeded.

Pytest looks for functions starting with "test_"

Suggested change
# File paths for the input and output PDFs
input_pdf = Path("test_input.pdf")
output_pdf = Path("test_output.pdf")
# Create the test PDF
create_test_pdf(input_pdf)
# List of layouts to test
layouts = ["2x2", "3x3", "1x2", "2x1"]
for layout in layouts:
print(f"\nTesting layout: {layout}")
up2_main(input_pdf, output_pdf, layout=layout)
# Read the output PDF and print the number of pages
reader = PdfReader(str(output_pdf))
print(f"Output PDF for layout {layout} has {len(reader.pages)} pages.")
# Clean up
if output_pdf.exists():
os.remove(output_pdf)
if input_pdf.exists():
os.remove(input_pdf)
def test_layout_option():
# File paths for the input and output PDFs
input_pdf = Path("test_input.pdf")
output_pdf = Path("test_output.pdf")
# Create the test PDF
create_test_pdf(input_pdf)
# List of layouts to test
layouts = ["2x2", "3x3", "1x2", "2x1"]
for layout in layouts:
print(f"\nTesting layout: {layout}")
up2_main(input_pdf, output_pdf, layout=layout)
# Read the output PDF and print the number of pages
reader = PdfReader(str(output_pdf))
print(f"Output PDF for layout {layout} has {len(reader.pages)} pages.")
# Clean up
if output_pdf.exists():
os.remove(output_pdf)
if input_pdf.exists():
os.remove(input_pdf)

writer.add_page(page)

with open(output, "wb") as f_out:
writer.write(f_out)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
writer.write(f_out)
writer.write(f_out)

Comment on lines +38 to +44
# Define layout configurations (columns, rows) for each grid type
layout_options = {
"2x2": (2, 2),
"3x3": (3, 3),
"1x2": (1, 2),
"2x1": (2, 1)
}
Copy link
Member

Choose a reason for hiding this comment

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

What is the reason to restrict the layout options? Wouldn't any XxY with X and Y being positive integers work?

@MartinThoma MartinThoma changed the title ENH: Add Flexible PDF Layout Options with --layout Parameter ENH: Add --layout parameter to up2 Dec 8, 2024
@MartinThoma
Copy link
Member

Thank you for your contribution!

@MartinThoma
Copy link
Member

Could you please run black . (an auto-formatter; you might need to install it via pip install black first)?

Copy link
Member

Choose a reason for hiding this comment

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

You could add this test to test_up2.py

@MartinThoma
Copy link
Member

@mulla028 Sorry that it took me a month to review 🙈 If you want, I would clean-up the remaining parts. Just let me know if you want to finish it yourself or not :-)

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.

ENH: pdfly x2pdf --layout
2 participants