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

509 ignore blank patches #540

Merged
merged 9 commits into from
Dec 19, 2024

Conversation

mradamcox
Copy link
Contributor

@mradamcox mradamcox commented Dec 14, 2024

Summary

This PR adds a boolean flag skip_blank_patches to patchify_all() (which is passed to _patchify_by_pixel()) that allows users to ignore patches that are entirely blank, i.e. all pixels within the patch have a value of 0.

Default value of the flag is False so that existing behavior will stay the same.

Fixes #509

Describe your changes

The evaluation on the patch is simple, it just uses pillow's Image.get_bbox() and if None is returned the patch is assumed "empty".

I've added a pair of tests and images to test against, one RGB and one RGB with an alpha channel. The latter may not really be necessary... I realized that even if the map image has a "valid" patch that is all black (for example, a patch that is not around the collar because of a tilted image, but just within a black spot in the middle of the map), then that patch won't be useful for text-spotting anyway and can safely be skipped.

Checklist before assigning a reviewer (update as needed)

  • Self-review code
  • Ensure submission passes current tests
  • Add tests
  • Update relevant docs
  • Update changelog

I'll set this to draft for now, and can add the docs and changelog once we confirm everything looks good and this is the right approach.

Reviewer checklist

Please add anything you want reviewers to specifically focus/comment on.

  • Everything looks ok?

@rwood-97
Copy link
Collaborator

Hi, I noticed some of our tests weren't running on your PR so I've just updated the triggers for some of our workflows. Could you merge main to pull these updates in and hopefully all tests will run now

@mradamcox
Copy link
Contributor Author

Ok I've rebased main into this branch.

Copy link

codecov bot commented Dec 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Flag Coverage Δ
unittests 77.82% <100.00%> (+0.16%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
mapreader/load/images.py 88.58% <100.00%> (+0.05%) ⬆️
tests/test_load/test_images.py 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

@mradamcox mradamcox marked this pull request as ready for review December 18, 2024 16:35
Copy link
Contributor

@kallewesterling kallewesterling left a comment

Choose a reason for hiding this comment

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

This looks simple enough of an edit to me, and seems to contribute great functionality. I'll let @rwood-97 do the formal approve/request changes, but LGTM! Thank you so much for the contribution, @mradamcox!

Copy link
Collaborator

@rwood-97 rwood-97 left a comment

Choose a reason for hiding this comment

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

I made one minor update just where something had been missed from the docs but otherwise looks great

@rwood-97 rwood-97 merged commit ae10e4b into maps-as-data:main Dec 19, 2024
9 checks passed
@mradamcox mradamcox deleted the 509_ignore_blank_patches branch December 19, 2024 15:59
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.

Suggestion: Ignore patches that are completely empty
3 participants