Skip to content

Conversation

TheMathix
Copy link

@TheMathix TheMathix commented Aug 26, 2025

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:

  • Add apply_to_images method to HueSaturationValue for batch HSV shifts
  • Support arbitrary leading batch dimensions in shift_hsv

Enhancements:

  • Refactor shift_hsv to use vectorized reshape and OpenCV conversions for improved performance
  • Add input validation for dimensions and channel counts in shift_hsv
  • Handle grayscale inputs by expanding to RGB, then restoring single-channel output

Tests:

  • Add parameterized tests for batched hue, saturation, and value adjustments across grayscale and RGB inputs in uint8 and float32

Copy link

sourcery-ai bot commented Aug 26, 2025

Reviewer's Guide

Refactors 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_images

sequenceDiagram
    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
Loading

File-Level Changes

Change Details Files
Support for batched inputs and rigorous shape/Channel validation
  • Added ndim and channel count checks
  • Extracted H, W, C from trailing axes
  • Implemented fast path when no shifts are requested
  • Treated C==1 as grayscale batch/single
functional.py
Vectorized color-space conversion and per-channel adjustments
  • Collapsed leading batch dims before cv2.cvtColor
  • Reshaped flattened RGB to HSV and back
  • Applied hue LUT and non-inplace saturation/value shifts
  • Stacked/unstacked HSV channels without Python loops
functional.py
Output reshaping to preserve original format
  • Reshaped processed data back to leading dims
  • Converted back to grayscale when input was single-channel
  • Ensured output has correct channel-last shape
functional.py
Batch-aware API extension in transform class
  • Added apply_to_images method forwarding to functional.shift_hsv
  • Allowed passing arbitrary params through to functional layer
transforms.py
New tests for batched HueSaturationValue
  • Parameterized single- and multi-channel, uint8/float32 batches
  • Compared batch outputs against per-image apply results
  • Included sanity checks for actual transformations
tests/test_transforms.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!

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

@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 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>

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.

Comment on lines +115 to +116
lut_hue = np.arange(256, dtype=np.int16)
lut_hue = np.mod(lut_hue + int(hue_shift), 180).astype(np.uint8)
Copy link

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.

@ternaus
Copy link
Contributor

ternaus commented Aug 27, 2025

@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

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