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 BboxProcessor filtering on max accept ratio #2313

Conversation

CristoJV
Copy link
Contributor

@CristoJV CristoJV commented Jan 28, 2025

Description

Issue

In the current implementation, the parameter max_accept_ratio is not explicitly passed in the BboxProcessor.filter function

def filter(self, data: np.ndarray, shape: ShapeType) -> np.ndarray:
self.params: BboxParams
return filter_bboxes(
data,
shape,
min_area=self.params.min_area,
min_visibility=self.params.min_visibility,
min_width=self.params.min_width,
min_height=self.params.min_height,
)

This omission causes the default value (None) for max_accept_ratio to be used, making BboxProcessor effectively ignore bounding-box filtering by aspect ratio.

Fix

The parameter max_accept_ratio is now explicitly passed in both BboxProcessor, ensuring the correct behavior when filtering bounding boxes based on their aspect ratio.

def filter(self, data: np.ndarray, shape: ShapeType) -> np.ndarray:
    self.params: BboxParams
    return filter_bboxes(
        data,
        shape,
        min_area=self.params.min_area,
        min_visibility=self.params.min_visibility,
        min_width=self.params.min_width,
        min_height=self.params.min_height,
+       max_accept_ratio=self.params.max_accept_ratio
    )

Related


Tests Added

New test cases have been introduced to verify that max_accept_ratio is used in different scenarios, including:

  1. test_bbox_processor_max_accept_ratio
    Verifies that BboxProcessor.preprocess and BboxProcessor.postprocess respect the max_accept_ratio.

  2. test_compose_with_max_accept_ratio
    Ensures bounding boxes are filtered correctly when using Albumentations Compose.

  3. test_compose_max_accept_ratio_all_formats
    Confirms max_accept_ratio filtering works for coco, pascal_voc, yolo, and albumentations formats.

Test Details

  • Environment: Python 3.12.8, Ubuntu 24.02, Docker
  • All tests have passed successfully, validating the fix.

Additional Notes

  • This change is backward-compatible.
  • Filtering now behaves consistently across different bounding-box operations when max_accept_ratio is specified.

Summary by Sourcery

Fix filtering of bounding boxes by aspect ratio using the max_accept_ratio parameter in BboxProcessor.

Bug Fixes:

  • Fixed a bug where the max_accept_ratio parameter was not being used in the BboxProcessor, resulting in incorrect filtering of bounding boxes.

Tests:

  • Added tests to verify the correct behavior of max_accept_ratio in BboxProcessor.preprocess, BboxProcessor.postprocess, and Compose with different bounding box formats.

Copy link
Contributor

sourcery-ai bot commented Jan 28, 2025

Reviewer's Guide by Sourcery

The pull request addresses an issue where the max_accept_ratio parameter was not being passed to the filter_bboxes function within BboxProcessor. This resulted in bounding box filtering by aspect ratio being ignored. The fix ensures that max_accept_ratio is correctly passed, enabling proper filtering. Additionally, new test cases were added to validate the fix.

Sequence diagram for BboxProcessor filtering with max_accept_ratio

sequenceDiagram
    participant Client
    participant BboxProcessor
    participant filter_bboxes

    Client->>BboxProcessor: filter(data, shape)
    Note over BboxProcessor: Now includes max_accept_ratio
    BboxProcessor->>filter_bboxes: filter_bboxes(data, shape, ...)
    filter_bboxes-->>BboxProcessor: filtered bounding boxes
    BboxProcessor-->>Client: filtered bounding boxes
Loading

Class diagram showing BboxProcessor changes

classDiagram
    class BboxProcessor {
        +params: BboxParams
        +filter(data: np.ndarray, shape: ShapeType) np.ndarray
    }
    class BboxParams {
        +min_area: float
        +min_visibility: float
        +min_width: float
        +min_height: float
        +max_accept_ratio: float
    }
    BboxProcessor --> BboxParams: uses
    note for BboxProcessor "Now correctly passes max_accept_ratio"
Loading

Flow diagram of bbox filtering process

flowchart TD
    A[Input Bounding Box] --> B{Check Filters}
    B -->|Apply min_area| C{Check min_area}
    B -->|Apply min_visibility| D{Check min_visibility}
    B -->|Apply min_width| E{Check min_width}
    B -->|Apply min_height| F{Check min_height}
    B -->|Apply max_accept_ratio| G{Check max_accept_ratio}
    C & D & E & F & G -->|Pass| H[Keep Box]
    C & D & E & F & G -->|Fail| I[Filter Out Box]
    H & I --> J[Return Filtered Boxes]
Loading

File-Level Changes

Change Details Files
Pass max_accept_ratio to filter_bboxes
  • The max_accept_ratio parameter is now explicitly passed to the filter_bboxes function in the BboxProcessor.filter method.
albumentations/core/bbox_utils.py
Add new test cases for max_accept_ratio
  • Added test_filter_bboxes_aspect_ratio to verify filter_bboxes with different max_accept_ratio values.
  • Added test_bbox_processor_max_accept_ratio to verify BboxProcessor with different max_accept_ratio values.
  • Added test_compose_with_max_accept_ratio to verify Compose with different max_accept_ratio values.
  • Added test_compose_max_accept_ratio_all_formats to verify Compose with different max_accept_ratio values and bbox formats.
tests/test_bbox.py

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!
  • Generate a plan of action for an issue: Comment @sourcery-ai plan on
    an issue to generate a plan of action for it.

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @CristoJV - I've reviewed your changes and they look great!

Here's what I looked at during the review
  • 🟢 General issues: all looks good
  • 🟢 Security: all looks good
  • 🟡 Testing: 1 issue found
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

tests/test_bbox.py Show resolved Hide resolved
@ternaus ternaus merged commit d5113d5 into albumentations-team:main Jan 28, 2025
8 of 14 checks passed
@ternaus
Copy link
Collaborator

ternaus commented Jan 28, 2025

@CristoJV Thank you for the catch

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