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] ElementList filter on visualise #256

Merged
merged 2 commits into from
Oct 12, 2021
Merged

[FIX] ElementList filter on visualise #256

merged 2 commits into from
Oct 12, 2021

Conversation

mcrts
Copy link

@mcrts mcrts commented Oct 6, 2021

Description

When using a ElementList to filter Elements visualised by the visualise function, no filter is applied and all elements are displayed. The proposed changes, address this issue by performing the intersection between the pages' ElementList and the ElementList passed as argument. See #255 for details.

Linked issues
Closes #255

Testing

The changes were tested visually.

from py_pdf_parser.loaders import load_file
from py_pdf_parser.visualise import visualise
document = load_file("path_to_file.pdf")
visualise(document, elements=document.elements[0::2]) #should only display every other element

Checklist

  • I have provided a good description of the change above
  • I have added any necessary tests
  • I have added all necessary type hints
  • I have checked my linting (docker-compose run --rm lint)
  • I have added/updated all necessary documentation
  • I have updated CHANGELOG.md, following the format from
    Keep a Changelog.

Copy link
Owner

@jstockwin jstockwin left a comment

Choose a reason for hiding this comment

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

Hey @mcrts, thanks for taking the time to submit such a detailed issue and this fix - much appreciated.

However, I think the fix isn't quite right due to the subtlety of and vs &. I've suggested a change which should solve it.

This issue as a whole shows the importance of #219...

py_pdf_parser/visualise/main.py Outdated Show resolved Hide resolved
Copy link
Owner

@jstockwin jstockwin left a comment

Choose a reason for hiding this comment

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

In addition, please do update the changelog with this fix

Copy link
Owner

@jstockwin jstockwin left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for the fix @mcrts! I'll try to get that into a new release in the next few days.

@jstockwin jstockwin merged commit cd0be71 into jstockwin:master Oct 12, 2021
@jstockwin
Copy link
Owner

This fix is included in release 0.10.1.

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.

ElementList filter on visualise function does not work
2 participants