-
Notifications
You must be signed in to change notification settings - Fork 19.5k
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
[Bounding Boxes] Add Support for Bounding Box Transformations during Image Resizing #20368
Conversation
Sample notebook of using |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR!
keras/src/layers/preprocessing/image_preprocessing/base_image_preprocessing_layer.py
Outdated
Show resolved
Hide resolved
keras/src/layers/preprocessing/image_preprocessing/bounding_boxes/converters_new.py
Outdated
Show resolved
Hide resolved
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #20368 +/- ##
==========================================
- Coverage 78.87% 73.30% -5.58%
==========================================
Files 512 516 +4
Lines 49266 49563 +297
Branches 7953 7962 +9
==========================================
- Hits 38861 36333 -2528
- Misses 8546 11420 +2874
+ Partials 1859 1810 -49
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
keras/src/layers/preprocessing/image_preprocessing/base_image_preprocessing_layer.py
Outdated
Show resolved
Hide resolved
keras/src/layers/preprocessing/image_preprocessing/bounding_boxes/bounding_box.py
Show resolved
Hide resolved
keras/src/layers/preprocessing/image_preprocessing/base_image_preprocessing_layer.py
Outdated
Show resolved
Hide resolved
Remove input dict args for default orig_height and orig_width
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good, thanks for the updates! Please add a unit test for Resizing
bounding box support.
Change layer names Bounding box resizing as per the aspect ratio
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code looks good to me! Not sure about the max bbox layer.
A side question is whether we need "bbox" in the name of the transformations like affine_transform
etc et distinguish them from keras.ops.image.affine_transform
and so on. No strong opinion.
keras/src/layers/preprocessing/image_preprocessing/max_bounding_box.py
Outdated
Show resolved
Hide resolved
keras/src/layers/preprocessing/image_preprocessing/max_bounding_box.py
Outdated
Show resolved
Hide resolved
keras/src/layers/preprocessing/image_preprocessing/max_bounding_box.py
Outdated
Show resolved
Hide resolved
Can you take a look at the test failures? I think they are likely related to the fact that torch tests are running with |
Oh yes!! thanks I updated the test cases for Reference: |
I guess it should be fine as user will use |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you for the great contribution!
This PR covers below cases:
orig_height
andorig_width
for bounding boxes transformation as each image can have separate bounding boxes relative to the original image size. Later we can use the same to transform bounding boxes as per the ratios.convert_format
to be compatible intf.data
pipeline regardless of backend.Just a temporary file without disturbing previous implementation of
convert_format