-
-
Notifications
You must be signed in to change notification settings - Fork 11
Feat/apply to images hue saturation value #79
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
base: main
Are you sure you want to change the base?
Feat/apply to images hue saturation value #79
Conversation
Reviewer's GuideRefactors shift_hsv to handle both single images and batches in a vectorized manner with shape validation, adds a batch-aware apply_to_images API, and covers the changes with new tests. Sequence diagram for batch image HSV shift using apply_to_imagessequenceDiagram
participant User
participant SomeTransformClass
participant shift_hsv
User->>SomeTransformClass: call apply_to_images(images, hue_shift, sat_shift, val_shift)
SomeTransformClass->>shift_hsv: shift_hsv(images, hue_shift, sat_shift, val_shift)
shift_hsv-->>SomeTransformClass: return batch-shifted images
SomeTransformClass-->>User: return result
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey there - I've reviewed your changes - here's some feedback:
- Consider refactoring the batch flatten/unflatten logic into a shared helper to reduce duplication and make the code more readable.
- Add an explicit contiguity and dtype normalization step (e.g. via np.ascontiguousarray or casting) before cv2.cvtColor to avoid surprises with float32 or non-contiguous inputs.
- Clarify or enforce the expected value range for float images (0–1 vs 0–255) so HSV↔RGB conversions remain consistent and predictable across dtypes.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider refactoring the batch flatten/unflatten logic into a shared helper to reduce duplication and make the code more readable.
- Add an explicit contiguity and dtype normalization step (e.g. via np.ascontiguousarray or casting) before cv2.cvtColor to avoid surprises with float32 or non-contiguous inputs.
- Clarify or enforce the expected value range for float images (0–1 vs 0–255) so HSV↔RGB conversions remain consistent and predictable across dtypes.
## Individual Comments
### Comment 1
<location> `albumentations/augmentations/pixel/functional.py:115` </location>
<code_context>
if hue_shift != 0:
- lut_hue = np.arange(0, 256, dtype=np.int16)
- lut_hue = np.mod(lut_hue + hue_shift, 180).astype(np.uint8)
+ lut_hue = np.arange(256, dtype=np.int16)
+ lut_hue = np.mod(lut_hue + int(hue_shift), 180).astype(np.uint8)
hue = sz_lut(hue, lut_hue, inplace=False)
</code_context>
<issue_to_address>
Casting hue_shift to int may lose precision if float values are passed.
If float values are valid for hue_shift, casting to int may cause unintended results. Consider rounding or clarify in documentation that only integers are supported.
</issue_to_address>
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
lut_hue = np.arange(256, dtype=np.int16) | ||
lut_hue = np.mod(lut_hue + int(hue_shift), 180).astype(np.uint8) |
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.
nitpick: Casting hue_shift to int may lose precision if float values are passed.
If float values are valid for hue_shift, casting to int may cause unintended results. Consider rounding or clarify in documentation that only integers are supported.
…TheMathix/AlbumentationsX into feat/apply_to_images-HueSaturationValue
@TheMathix You need to set up pre-commit hook, so that you would not be able to commit code, without it passing. https://albumentations.ai/docs/contributing/environment-setup/#4-set-up-pre-commit-hooks |
Followed by the instructions here by @ternaus I vectorized the function instead of calling apply in a loop
Summary by Sourcery
Enable efficient batched hue, saturation, and value adjustments by vectorizing shift_hsv and exposing a batch API in the HueSaturationValue transform; add tests to verify consistency between single-image and batch processing.
New Features:
Enhancements:
Tests: