Skip to content

ENH: With extract-annotated-pages command#98

Merged
Lucas-C merged 2 commits intopy-pdf:mainfrom
wolfram77:main
Feb 17, 2025
Merged

ENH: With extract-annotated-pages command#98
Lucas-C merged 2 commits intopy-pdf:mainfrom
wolfram77:main

Conversation

@wolfram77
Copy link
Copy Markdown
Contributor

This pull request addresses the following issue:

ENH: Add command to extract annotated pages #97


@Lucas-C

  • even links (internal or external) are annotations!

This was the issue.

@wolfram77 wolfram77 changed the title With extract-annotated-pages command Feb 6, 2025
@Lucas-C
Copy link
Copy Markdown
Member

Lucas-C commented Feb 7, 2025

Thank you for your contribution @wolfram77 👍

Could you please address the following points, and I'll be happy to merge your PR:

  • please run black pdfly/extract_annotated_pages.py so that the GitHub Actions CI pipeline passes
  • please add a mention of your addition to CHANGELOG.md as part of this PR
  • please include at lease one basic unit test in test/test_extract_annotations.py. You could take inspiration from the code snippet below to create a PDF file with annotations "on the fly" in this unit test:
from fpdf import FPDF

pdf = FPDF()
pdf.set_font("Helvetica", size=12)

pdf.add_page()
text = "Link set over an arbitrary area with FPDF.link()"
x, y = 20, 150
pdf.text(x=x, y=y, text=text)
width = pdf.get_string_width(text)
pdf.link(
    x=x,
    y=y - pdf.font_size,
    w=width,
    h=pdf.font_size,
    link="https://github.com/py-pdf/fpdf2/discussions",
)

pdf.add_page()
pdf.text_annotation(
    x=20,
    y=150,
    text=f"This is a default text annotation.",
)
pdf.output("pdfly_pr_98.pdf")

PS: I'll be on holiday for a few days, so I'll get back to you only mid-february.

@Lucas-C
Copy link
Copy Markdown
Member

Lucas-C commented Feb 7, 2025

@all-contributors please add @wolfram77 for code

@allcontributors
Copy link
Copy Markdown
Contributor

@Lucas-C

I've put up a pull request to add @wolfram77! 🎉

@wolfram77
Copy link
Copy Markdown
Contributor Author

@Lucas-C Thanks for reviewing the PR. To simplify the test, I added a yellow highlight to page 7 of resources/input8.pdf. The test case now looks for one annotated page in it.

@Lucas-C
Copy link
Copy Markdown
Member

Lucas-C commented Feb 7, 2025

Nice, good job with the unit test 👍

I think there are issues with the unit tests, on the main branch, regardless of this PR.

I won't have the time to fix them today, so you can either have a look at it based on the GitHub Actions logs, or else I'll fix that when I'll be back from holiday.

@wolfram77
Copy link
Copy Markdown
Contributor Author

wolfram77 commented Feb 7, 2025

@Lucas-C From the first failed test, I see the following:

        # Assert
        captured = capsys.readouterr()
>       assert exit_code == 0, captured
E       AssertionError: CaptureResult(out='', err="\x1b[33mUsage: \x1b[0mpytest update-offsets [OPTIONS] FILE_IN
E         \x1b[2mTry \x1b[0m\x1b[2;34m'pytest update-offsets \x1b[0m\x1b[1;2;34m-\x1b[0m\x1b[1;2;34m-help\x1b[0m\x1b[2;34m'\x1b[0m\x1b[2m for help.\x1b[0m
E         \x1b[31m╭─\x1b[0m\x1b[31m Error \x1b[0m\x1b[31m─────────────────────────────────────────────────────────────────────\x1b[0m\x1b[31m─╮\x1b[0m
E         \x1b[31m│\x1b[0m Got unexpected extra argument                                                \x1b[31m│\x1b[0m
E         \x1b[31m│\x1b[0m (/tmp/pytest-of-runner/pytest-0/test_update_offsets0/file-with-offsets-out.p \x1b[31m│\x1b[0m
E         \x1b[31m│\x1b[0m df)                                                                          \x1b[31m│\x1b[0m
E         \x1b[31m╰──────────────────────────────────────────────────────────────────────────────╯\x1b[0m
E         ")
E       assert 2 == 0

I am not sure why it seems to expect update-offsets to have only FILE_IN as an argument (no FILE_OUT), so it is failing with Got unexpected extra argument error. Could it be a pytest / typer version issue? Would be best you take a look at it when you are back.

@Lucas-C
Copy link
Copy Markdown
Member

Lucas-C commented Feb 7, 2025

I fixed the main branch.

mypy reports some minor issues with your PR:

pdfly/extract_annotated_pages.py:20: error: "Path" has no attribute "with_stem"  [attr-defined]
pdfly/extract_annotated_pages.py:28: error: "PdfObject" has no attribute "__iter__" (not iterable)  [attr-defined]
pdfly/cli.py:348: error: Argument 2 to "main" has incompatible type "Optional[Path]"; expected "Path"  [arg-type]
@wolfram77
Copy link
Copy Markdown
Contributor Author

@Lucas-C please update me when you are back. I have (hopefully) addressed the issues you mentioned above.

@Lucas-C
Copy link
Copy Markdown
Member

Lucas-C commented Feb 17, 2025

@wolfram77 I updated your branch to rebase it and fix the last issue with ruff

@Lucas-C Lucas-C force-pushed the main branch 2 times, most recently from ca9000d to 505ab93 Compare February 17, 2025 15:40
Seems that the `| None` syntax is not valid with Python 3.8

Co-authored-by: Lucas Cimon <925560+Lucas-C@users.noreply.github.com>
@Lucas-C Lucas-C merged commit 407e6cd into py-pdf:main Feb 17, 2025
10 checks passed
@Lucas-C
Copy link
Copy Markdown
Member

Lucas-C commented Feb 17, 2025

Merged!

Thank you @wolfram77 👍 🙂

@wolfram77
Copy link
Copy Markdown
Contributor Author

Lucas-C added a commit that referenced this pull request Feb 19, 2025
Co-authored-by: Lucas Cimon <925560+Lucas-C@users.noreply.github.com>
Lucas-C added a commit that referenced this pull request Feb 19, 2025
Co-authored-by: Lucas Cimon <925560+Lucas-C@users.noreply.github.com>
@Lucas-C
Copy link
Copy Markdown
Member

Lucas-C commented Oct 13, 2025

This new command has been included in the latest 0.5.0 release: https://github.com/py-pdf/pdfly/releases/tag/0.5.0

@wolfram77
Copy link
Copy Markdown
Contributor Author

It has been long time. I hope it helps the users of PDF fly :) ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants