Skip to content

Return None When There Isn't Meaningful Letterboxing#75

Open
mjs2600 wants to merge 2 commits intomainfrom
feature/unletterbox-none
Open

Return None When There Isn't Meaningful Letterboxing#75
mjs2600 wants to merge 2 commits intomainfrom
feature/unletterbox-none

Conversation

@mjs2600
Copy link
Contributor

@mjs2600 mjs2600 commented Feb 25, 2026

The current implementation will return the original image if there isn't any meaningful letterboxing around the image. This PR changes that so that the caller knows if an image was modified or not.

The current implementation will return the original image if there isn't any meaningful letterboxing around the image. This PR changes that so that the caller knows if an image was modified or not.
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR changes the behavior of the unletterbox function to return None when no meaningful letterboxing is detected, instead of returning the full image bounds. This allows callers to distinguish between images that were actually modified versus those that didn't need processing.

Changes:

  • Modified unletterbox to return None when letterboxing is not detected or not meaningful
  • Updated load_and_preprocess to handle the new None return value correctly
  • Updated tests to expect None instead of full image bounds for unchanged images

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
perception/hashers/tools.py Changed three return statements from returning full image bounds (0, w), (0, h) to returning None, and updated documentation
perception/local_descriptor_deduplication.py Updated load_and_preprocess to check if unletterbox returns None and only crop if bounds are returned; removed unnecessary parentheses in lambda
tests/test_tools.py Updated tests to expect None instead of full image bounds; added formatting improvements; contains a critical bug in test_ffmpeg_video

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@mjs2600 mjs2600 requested review from emma-green, julia-thorn and sonali-thorn and removed request for sonali-thorn February 25, 2026 21:42
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