ENH: Add `--format` Option for Custom Page Sizes in `x2pdf` Command #65

mulla028 · 2024-10-30T02:48:30Z

Closes #63

Summary

This pull request introduces the --format option to the x2pdf command in pdfly, allowing users to specify custom page sizes for PDF output. The option supports standard formats such as Letter, A4-portrait, and A4-landscape, as well as custom dimensions (e.g., 210x297 mm). This enhancement provides flexibility for generating PDFs with various page sizes, scaling and centering images to fit each specified format.

Changes

Code Updates

cli.py
- Added --format option to the x2pdf command with a default value of A4-portrait.
- Updated help documentation to list available format options.
x2pdf.py
- Implemented the get_page_size function to map standard format names to dimensions or parse custom dimensions (e.g., 210x297).
- Updated image_to_pdf function to center and scale images according to the specified page size.
- Modified main function to accept format as a parameter and apply the selected page size to each PDF page.

Tests

test_x2pdf.py
- Added new test cases to validate the --format option with various formats, including standard (Letter, A4-portrait, A4-landscape) and custom dimensions (210x297).
- Included a test to handle invalid format values, verifying appropriate error handling.

Verification Steps

Run pdfly x2pdf --help to verify that --format is listed in the options.

Test x2pdf with standard and custom formats:

pdfly x2pdf sample-files/sample1.jpg --output output_letter.pdf --format Letter
pdfly x2pdf sample-files/sample2.jpg --output output_a4.pdf --format A4-portrait
pdfly x2pdf sample-files/sample3.jpg --output output_custom.pdf --format 210x297

pdfly/cli.py

pdfly/x2pdf.py

tests/test_x2pdf.py

Lucas-C · 2024-10-30T12:32:15Z

tests/test_x2pdf.py

+        if format_option != "invalid-format":
+            assert exit_code == 0, captured
+            assert captured.out == ""
+            assert output.exists()


It would be interesting to also validate the resulting pages dimensions.

This can be checked with pdfly pagemeta $pdf_filepath $page_index

Or using the underlying pypdf library: PdfReader(pdf: Path).mediabox/.cropbox/.artbox/.bleedbox

I think you did not handle that feedback comment @mulla028 🙂

pdfly/x2pdf.py

Lucas-C

Good job tackling this issue! 👍

Thank you for your contribution 🙂

I made a few comments as feedback.

mulla028 · 2024-10-30T16:10:11Z

Good job tackling this issue! 👍

Thank you for your contribution 🙂

I made a few comments as feedback.

Thank you for your feedback! I will do my best to make all these changes 🙂

mulla028 · 2024-10-30T18:11:06Z

@Lucas-C I tried to make all the requested changes. Review please, and let me know if you noticed any issues!

pdfly/cli.py

pdfly/x2pdf.py

…leted

…non-zero code and adjusting success message if issue occur.

…ling.

mulla028 · 2024-10-31T17:05:58Z

@Lucas-C
Thank you for your feedback, some things were pretty new to me. Thank you for your patience. Hopefully, this time code follows your requirements 😄

mulla028 · 2024-11-01T18:39:04Z

@Lucas-C is my problem comes with my code styling?

Lucas-C · 2024-11-01T20:21:37Z

pdfly/x2pdf.py

+            elif orientation == "portrait":
+                return (width, height)


Suggested change

elif orientation == "portrait":

return (width, height)

Just a minor code improvement suggestion.

This change is not mandatory before merging this PR.

Lucas-C · 2024-11-01T20:53:48Z

@Lucas-C is my problem comes with my code styling?

No, it's OK 🙂

I only see a couple of blocking points:

this comment has not been adressed yet: ENH: Add --format Option for Custom Page Sizes in x2pdf Command #65 (review)
the GitHub Actions pipeline is failing: https://github.com/py-pdf/pdfly/actions/runs/11616493703/job/32381813640?pr=65
This is due to a fail with the new unit test:

FAILED tests/test_x2pdf.py::test_x2pdf_with_format - ValueError: Invalid or unsupported page format provided: Letter

You should be able to reproduce the problem by executing the unit tests on your own computer 🙂

Lucas-C · 2024-11-20T12:35:21Z

Hi @mulla028 🙂

Just to get a quick update: do you still want to implement this feature?

mulla028 · 2024-11-20T16:32:37Z

Hi @mulla028 🙂

Just to get a quick update: do you still want to implement this feature?

Hi! Yes, I just had to do a lot of work for school. I am gonna finish implementing this issue by the end of this week.

Lucas-C · 2024-12-02T15:48:48Z

Hi @mulla028

I'm sorry to ask again, but it has been 2 weeks, an another contributor has submitted a PR for a several feature: #77

Do you think that you will have some time in the near future to finish implementing this?

Else we could consider having someone else finishing the work on this PR, either me, @pastor-robert or another contributor 🙂

mulla028 added 3 commits October 29, 2024 22:42

Introduced option to command. Udated help text to reflect new option.

fcf7aa2

Add support for custom page sizes in x2pdf with --format otion handling.

92a2c93

Add test for --format option in x2pdf to validate custom page sizes.

7eab975