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.

Sign up for GitHub

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

Jump to bottom

WIP: support font CMAP to translate chars with TJ operator #858

Closed
wants to merge 2 commits into from
Closed

WIP: support font CMAP to translate chars with TJ operator #858

wants to merge 2 commits into from

Conversation

VictorCarlquist
Copy link

@VictorCarlquist VictorCarlquist commented May 3, 2022 •
edited
Loading

Hello!

With this PR is possible to extractText from TJ operator, fixing the issues #576 and #654

It's adding support to CMAP translate when the PDF uses ToUnicode.

I need to add some tests on it yet.

Edit:

the issue #654 isn't fixed, I tested with the first link described in that issue, but the second link isn't returning text. I'm debug it, seems that the cmap table contains chars and bytes, if we convert the entire table to byte it will work.

@codecov
Copy link

codecov bot commented May 3, 2022 •
edited
Loading

Codecov Report

Merging #858 (d2bb1a5) into main (1d14a86) will increase coverage by 0.00%.
The diff coverage is 82.19%.

@@           Coverage Diff           @@
##             main     #858   +/-   ##
=======================================
  Coverage   78.25%   78.26%           
=======================================
  Files          16       16           
  Lines        4346     4416   +70     
  Branches      821      846   +25     
=======================================
+ Hits         3401     3456   +55     
- Misses        758      768   +10     
- Partials      187      192    +5
Impacted Files Coverage Δ
PyPDF2/_page.py 76.19% <82.19%> (-0.31%) ⬇️
PyPDF2/generic.py 78.05% <0.00%> (+0.27%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1d14a86...d2bb1a5. Read the comment docs.

@MartinThoma
Copy link
Member

Very nice! I'm looking forward to this one!

@VictorCarlquist VictorCarlquist force-pushed the extract_text_TJ branch 3 times, most recently from 735ced6 to 15eaf74 Compare May 7, 2022 18:16
@MartinThoma
Copy link
Member

By the way: it might make more sense to point this pr to the 2.0.0-dev branch. I'll only add Bugfixes to the 1.x version. I hope to finish the move to 2.x this month.

@VictorCarlquist
Copy link
Author

Ok, nice! I was struggling with python2.7...

I'll change the code to support only python3.

@MartinThoma
Copy link
Member

@VictorCarlquist I appreciate your energy 😄 👍 But please be aware that it will take the rest of May to clean things up. You will get a lot of merge conflicts if you start working on this now.

@VictorCarlquist VictorCarlquist force-pushed the extract_text_TJ branch 5 times, most recently from e565a11 to a9e1668 Compare May 29, 2022 22:37
@VictorCarlquist
Copy link
Author

VictorCarlquist commented May 29, 2022 •
edited
Loading

This PR seems to do almost the same thing of the #881. @pubpub-zz and @MartinThoma you can confirm it, please. If so, we can close this PR.

@MartinThoma
Copy link
Member

I ran the benchmark which I also ran over pubpub-zz's pr.

I gave a 👍 where your current PR performs better:

https://arxiv.org/pdf/1601.03642 : 0.9438654353562005 -> 0.94
https://arxiv.org/pdf/1602.06541 : 0.8978933061501869 -> 0.90
https://arxiv.org/pdf/1707.09725 : 0.9100581720093184 -> 0.91
https://arxiv.org/pdf/2201.00021 : 0.9499215589133845 -> 0.92 <--- drop
https://arxiv.org/pdf/2201.00022 : 0.9102201679631884 -> 0.91
https://arxiv.org/pdf/2201.00029 : 0.0,
https://arxiv.org/pdf/2201.00037 : 0.9155486607869612 -> 0.92
https://arxiv.org/pdf/2201.00069 : 0.8980679211032767 -> 0.29 <---- crazy drop
https://arxiv.org/pdf/2201.00151 : 0.8859883219294902 -> 0.93 👍
https://arxiv.org/pdf/2201.00178 : 0.8927337030785306 -> 0.90
https://arxiv.org/pdf/2201.00200 : 0.9683510183687691 -> 0.94 <-- drop
https://arxiv.org/pdf/2201.00201 : 0.9747879942829919 -> 0.94 <- drop
https://arxiv.org/pdf/2201.00214 : 0.8850769765492426 -> 0.95 <--- huge improvement!
https://arxiv.org/pdf/GeoTopo-book : 0.7860457992901709 -> 0.78

@MartinThoma
Copy link
Member

The results are mixed. In some cases your PR leads to better results, in some cases #929 (comment) is better. I wouldn't close either for the moment.

I would rather merge #929 as the overall results look better. But I hope that we can combine the best of both PRs.

@pubpub-zz pubpub-zz mentioned this pull request May 30, 2022
Merged
@VictorCarlquist
Copy link
Author

Okay, I really appreciated that you ran the benchmark., thanks!. I'll investigate why occurred a crazy drop in one case.

@MartinThoma MartinThoma added the workflow-text-extraction From a users perspective, text extraction is the affected feature/workflow label Jun 1, 2022
@MartinThoma
Copy link
Member

@VictorCarlquist I've merged #924 and just restructured the code a bit. I think that the current main branch contains everything that was added with this PR (and some more things).

Do you want to check?

If you don't want to have a look, I would close this PR. I'll give you credit in the CHANGELOG for the next release for working on this feature.

@MartinThoma
Copy link
Member

You mentioned that this PR is fixing the issues #576 and #654 .

At least #576 is still an issue with the current main branch. Hence I'll leave this PR open.

MartinThoma added a commit that referenced this pull request Jun 6, 2022
@MartinThoma
REL: 2.1.0
4e44122 
The highlight of the 2.1.0 release is the most massive improvement to the
text extraction capabilities of PyPDF2 since 2016 🥳🎊 A very big thank you goes
to [pubpub-zz](https://github.com/pubpub-zz) who took a lot of time and
knowledge about the PDF format to finally get those improvements into PyPDF2.
Thank you 🤗💚

In case the new function causes any issues, you can use `_extract_text_old`
for the old functionality. Please also open a bug ticket in that case.

There were several people who have attempted to bring similar improvements to
PyPDF2. All of those were valuable. The main reason why they didn't get merged
is the big amount of open PRs / issues. pubpub-zz was the most comprehensive
PR which also incorporated the latest changes of PyPDF2 2.0.0.

Thank you to [VictorCarlquist](https://github.com/VictorCarlquist) for #858 and
[asabramo](https://github.com/asabramo) for #464 🤗

New Features (ENH):
-  Massive text extraction improvement (#924). Closed many open issues:
    - Exceptions / missing spaces in extract_text() method (#17) 🕺
      - Whitespace issues in extract_text() (#42) 💃
      - pypdf2 reads the hifenated words in a new line (#246)
    - PyPDF2 failing to read unicode character (#37)
      - Unable to read bullets (#230)
    - ExtractText yields nothing for apparently good PDF (#168) 🎉
    - Encoding issue in extract_text() (#235)
    - extractText() doesn't work on Chinese PDF (#252)
    - encoding error (#260)
    - Trouble with apostophes in names in text "O'Doul" (#384)
    - extract_text works for some PDF files, but not the others (#437)
    - Euro sign not being recognized by extractText (#443)
    - Failed extracting text from French texts (#524)
    - extract_text doesn't extract ligatures correctly (#598)
    - reading spanish text - mark convert issue (#635)
    - Read PDF changed from text to random symbols (#654)
    - .extractText() reads / as 1. (#789)
-  Update glyphlist (#947) - inspired by #464
-  Allow adding PageRange objects (#948)

Bug Fixes (BUG):
-  Delete .python-version file (#944)
-  Compare StreamObject.decoded_self with None (#931)

Robustness (ROB):
-  Fix some conversion errors on non conform PDF (#932)

Documentation (DOC):
-  Elaborate on PDF text extraction difficulties (#939)
-  Add logo (#942)
-  rotate vs Transformation().rotate (#937)
-  Example how to use PyPDF2 with AWS S3 (#938)
-  How to deprecate (#930)
-  Fix typos on robustness page (#935)
-  Remove scripts (pdfcat) from docs (#934)

Developer Experience (DEV):
-  Ignore .python-version file
-  Mark deprecated code with no-cover (#943)
-  Automatically create Github releases from tags (#870)

Testing (TST):
-  Text extraction for non-latin alphabets (#954)
-  Ignore PdfReadWarning in benchmark (#949)
-  writer.remove_text (#946)
-  Add test for Tree and _security (#945)

Code Style (STY):
-  black, isort, Flake8, splitting buildCharMap (#950)

Full Changelog: 2.0.0...2.1.0
@VictorCarlquist
Copy link
Author

@VictorCarlquist I've merged #924 and just restructured the code a bit. I think that the current main branch contains everything that was added with this PR (and some more things).

Do you want to check?

If you don't want to have a look, I would close this PR. I'll give you credit in the CHANGELOG for the next release for working on this feature.

At least #576 is still an issue with the current main branch. Hence I'll leave this PR open.

Thank you, I've test the case of #576, this PR doesn't fix that because the warning is still showing up. but it avoids that program to stop the text extract. The #924 has the same behavior, by the way, very good code.

We can close this PR.

Thanks!

@MartinThoma
Copy link
Member

Thank you for checking!

@MartinThoma MartinThoma closed this Jun 9, 2022
@pubpub-zz
Copy link
Collaborator

I will have a look to see to confirm it will match the corrections I'm working on

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
workflow-text-extraction From a users perspective, text extraction is the affected feature/workflow
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@VictorCarlquist @MartinThoma @pubpub-zz