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

Fix .paint_path bug noted in issue #473 #512

Merged
merged 7 commits into from
Oct 12, 2020

Conversation

jsvine
Copy link
Contributor

@jsvine jsvine commented Sep 26, 2020

See #473 for details. Focuses on the handling of non-rect quadrilaterals, the decomposition of complex (m.h) paths into subpaths, and assigning those subpaths the correct LTCurve/LTRect type.

Pull request

Thanks for improving pdfminer.six! Please include the following information to
help us discuss and merge this PR:

A description of why this PR is needed. What does it fix? What does it improve?

Please see #473 for details.

A summary of the things that this PR changes.

This PR partially rewrites PDFLayoutAnalyzer.paint_path(...), and also adds a test to tests/test_converter.py.

How Has This Been Tested?

See changes to tests/test_converter.py, with more context available in #473

Checklist

  • I have added tests that prove my fix is effective or that my feature
    works
  • (not applicable) I have added docstrings to newly created methods and classes
  • I have optimized the code at least one time after creating the initial
    version
  • (not applicable) I have updated the README.md or I am verified that this
    is not necessary
  • (not applicable) I have updated the readthedocs documentation or I
    verified that this is not necessary
  • I have added a consice human-readable description of the change to
    CHANGELOG.md

Focuses on the handling of non-rect quadrilaterals, the decomposition of
complex (m.*h)* paths into subpaths, and assigning those subpaths the
correct LTCurve/LTRect type.

Also adds a test for cases presented in issue pdfminer#473
pdfminer/converter.py Outdated Show resolved Hide resolved
pdfminer/converter.py Outdated Show resolved Hide resolved
tests/test_converter.py Show resolved Hide resolved
@pietermarsman
Copy link
Member

@jsvine I think the code looks good. The tests look really good :). I suggested some minor changes.

jsvine and others added 3 commits October 11, 2020 18:11
- Adjusts logic to adhere to if-elif-else rather than early returns.

- Shortens subpath detection/reprocessing step, using re.finditer().
pdfminer/converter.py Outdated Show resolved Hide resolved
@jsvine
Copy link
Contributor Author

jsvine commented Oct 12, 2020

Thanks, @pietermarsman. The reordering of the logic makes general sense to me, but tests/test_converter.py:test_paint_path_quadrilaterals now fails (see above).

@pietermarsman pietermarsman merged commit e83dd26 into pdfminer:develop Oct 12, 2020
@jsvine jsvine deleted the fix-paint-path branch October 19, 2020 23:29
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.

2 participants