feat(erasing): add grayscale fill mode#262
Open
federicopozzi33 wants to merge 6 commits into
Open
Conversation
Reviewer's GuideAdds a new grayscale fill mode to Erasing/coarse dropout, implementing grayscale conversion for cutout holes across images, volumes, and batches, wires it into BaseDropout/Erasing APIs, and covers it with tests and minor refactors to hole sampling and typing. Sequence diagram for grayscale fill mode in Erasing/BaseDropoutsequenceDiagram
actor User
participant Erasing as Erasing.transform
participant BaseDropout as BaseDropout.apply
participant Cutout as cutout
participant GrayHoles as grayscale_holes
participant GrayConv as to_gray_weighted_average
participant GrayMC as grayscale_to_multichannel
User->>Erasing: __call__(image, fill="grayscale")
Erasing->>BaseDropout: apply(img, holes, seed)
BaseDropout->>BaseDropout: validate fill == "grayscale" and channels in {1,3}
BaseDropout->>Cutout: cutout(img, holes, fill="grayscale", random_generator)
Cutout->>Cutout: detect fill == "grayscale"
Cutout->>GrayHoles: grayscale_holes(img, holes)
loop for each hole
GrayHoles->>GrayConv: to_gray_weighted_average(patch)
GrayConv-->>GrayHoles: grayscale_patch
GrayHoles->>GrayMC: grayscale_to_multichannel(grayscale_patch, 3)
GrayMC-->>GrayHoles: rgb_grayscale_patch
GrayHoles-->>GrayHoles: write back rgb_grayscale_patch
end
GrayHoles-->>Cutout: img_with_grayscale_holes
Cutout-->>BaseDropout: img_with_grayscale_holes
BaseDropout-->>Erasing: img_with_grayscale_holes
Erasing-->>User: transformed image
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The channel validation for grayscale/inpainting is duplicated in
apply,apply_to_images, andapply_to_volumes; consider extracting a small helper to centralize theget_num_channelslogic and error message so the behavior stays consistent across targets. - In
grayscale_holes/fill_volume_holes_with_grayscale/fill_volumes_holes_with_grayscale, the error message and docstrings say 1- or 3-channel images are expected, but the code only raises fornum_channels != 3after early-returning for 1; it would be clearer to check explicitly for{1, 3}and use a message that matches that condition. - You widened several
fillparameters fromLiteral[...]to plainstr(e.g., incutout,cutout_on_volume,cutout_on_volumes), which loses static type safety; consider keeping or reintroducing aLiteralunion that includes the new "grayscale" option rather than using barestr.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The channel validation for grayscale/inpainting is duplicated in `apply`, `apply_to_images`, and `apply_to_volumes`; consider extracting a small helper to centralize the `get_num_channels` logic and error message so the behavior stays consistent across targets.
- In `grayscale_holes` / `fill_volume_holes_with_grayscale` / `fill_volumes_holes_with_grayscale`, the error message and docstrings say 1- or 3-channel images are expected, but the code only raises for `num_channels != 3` after early-returning for 1; it would be clearer to check explicitly for `{1, 3}` and use a message that matches that condition.
- You widened several `fill` parameters from `Literal[...]` to plain `str` (e.g., in `cutout`, `cutout_on_volume`, `cutout_on_volumes`), which loses static type safety; consider keeping or reintroducing a `Literal` union that includes the new "grayscale" option rather than using bare `str`.
## Individual Comments
### Comment 1
<location path="tests/test_augmentations.py" line_range="1289-1313" />
<code_context>
assert np.all(result[:, :, channel_idx] == expected_values[channel_idx])
+def test_erasing_grayscale_fill_converts_only_sampled_patch():
+ image = TestDataFactory.create_image((64, 64, 3), dtype=np.uint8, seed=137)
+
+ transform = A.Erasing(
+ scale=(0.2, 0.2),
+ ratio=(1.0, 1.0),
+ fill="grayscale",
+ p=1.0,
+ )
+ transform.set_random_seed(137)
+
+ result = transform(image=image)["image"]
+ holes = transform.params["holes"]
+
+ assert holes.shape == (1, 4)
+ x_min, y_min, x_max, y_max = holes[0]
+
+ outside_mask = np.ones(image.shape[:2], dtype=bool)
+ outside_mask[y_min:y_max, x_min:x_max] = False
+
+ np.testing.assert_array_equal(result[outside_mask], image[outside_mask])
+
+ hole_patch = result[y_min:y_max, x_min:x_max]
+ np.testing.assert_array_equal(hole_patch[..., 0], hole_patch[..., 1])
+ np.testing.assert_array_equal(hole_patch[..., 1], hole_patch[..., 2])
+
+
</code_context>
<issue_to_address>
**suggestion (testing):** Consider asserting that the dtype is preserved when using `fill="grayscale"`.
To better pin down the behavior of this mode, please also assert that `result.dtype == image.dtype` (here `np.uint8`), so that any future change that alters the dtype (e.g., to float) will be caught by this test.
```suggestion
def test_erasing_grayscale_fill_converts_only_sampled_patch():
image = TestDataFactory.create_image((64, 64, 3), dtype=np.uint8, seed=137)
transform = A.Erasing(
scale=(0.2, 0.2),
ratio=(1.0, 1.0),
fill="grayscale",
p=1.0,
)
transform.set_random_seed(137)
result = transform(image=image)["image"]
holes = transform.params["holes"]
# Ensure that using fill="grayscale" does not change the dtype of the image
assert result.dtype == image.dtype
assert holes.shape == (1, 4)
x_min, y_min, x_max, y_max = holes[0]
outside_mask = np.ones(image.shape[:2], dtype=bool)
outside_mask[y_min:y_max, x_min:x_max] = False
np.testing.assert_array_equal(result[outside_mask], image[outside_mask])
hole_patch = result[y_min:y_max, x_min:x_max]
np.testing.assert_array_equal(hole_patch[..., 0], hole_patch[..., 1])
np.testing.assert_array_equal(hole_patch[..., 1], hole_patch[..., 2])
```
</issue_to_address>
### Comment 2
<location path="tests/test_augmentations.py" line_range="1341-1367" />
<code_context>
+ )
+
+
+def test_erasing_grayscale_fill_on_volume():
+ volume = TestDataFactory.create_volume((4, 64, 64, 3), dtype=np.uint8, seed=137)
+
+ transform = A.Erasing(
+ scale=(0.2, 0.2),
+ ratio=(1.0, 1.0),
+ fill="grayscale",
+ p=1.0,
+ )
+ transform.set_random_seed(137)
+
+ result = transform(volume=volume)["volume"]
+ holes = transform.params["holes"]
+
+ assert holes.shape == (1, 4)
+ x_min, y_min, x_max, y_max = holes[0]
+
+ result_outside = result.copy()
+ volume_outside = volume.copy()
+ result_outside[:, y_min:y_max, x_min:x_max, :] = 0
+ volume_outside[:, y_min:y_max, x_min:x_max, :] = 0
+
+ np.testing.assert_array_equal(result_outside, volume_outside)
+
+ hole_patch = result[:, y_min:y_max, x_min:x_max, :]
+ np.testing.assert_array_equal(hole_patch[..., 0], hole_patch[..., 1])
+ np.testing.assert_array_equal(hole_patch[..., 1], hole_patch[..., 2])
+
+
</code_context>
<issue_to_address>
**suggestion (testing):** Strengthen the volume test by verifying that the erased region actually changes relative to the input.
Currently the test only confirms that the outside region is unchanged and that the erased region is grayscale, but not that the grayscale values differ from the original volume. To guard against regressions where the grayscale fill becomes a no-op, add an assertion like `assert not np.array_equal(hole_patch, volume[:, y_min:y_max, x_min:x_max, :])` (or a similar pixel-level check) to ensure the erased patch is actually modified.
```suggestion
def test_erasing_grayscale_fill_on_volume():
volume = TestDataFactory.create_volume((4, 64, 64, 3), dtype=np.uint8, seed=137)
transform = A.Erasing(
scale=(0.2, 0.2),
ratio=(1.0, 1.0),
fill="grayscale",
p=1.0,
)
transform.set_random_seed(137)
result = transform(volume=volume)["volume"]
holes = transform.params["holes"]
assert holes.shape == (1, 4)
x_min, y_min, x_max, y_max = holes[0]
result_outside = result.copy()
volume_outside = volume.copy()
result_outside[:, y_min:y_max, x_min:x_max, :] = 0
volume_outside[:, y_min:y_max, x_min:x_max, :] = 0
np.testing.assert_array_equal(result_outside, volume_outside)
hole_patch = result[:, y_min:y_max, x_min:x_max, :]
volume_hole_patch = volume[:, y_min:y_max, x_min:x_max, :]
# Ensure the erased region actually changes relative to the original volume
assert not np.array_equal(hole_patch, volume_hole_patch)
np.testing.assert_array_equal(hole_patch[..., 0], hole_patch[..., 1])
np.testing.assert_array_equal(hole_patch[..., 1], hole_patch[..., 2])
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+1289
to
+1313
| def test_erasing_grayscale_fill_converts_only_sampled_patch(): | ||
| image = TestDataFactory.create_image((64, 64, 3), dtype=np.uint8, seed=137) | ||
|
|
||
| transform = A.Erasing( | ||
| scale=(0.2, 0.2), | ||
| ratio=(1.0, 1.0), | ||
| fill="grayscale", | ||
| p=1.0, | ||
| ) | ||
| transform.set_random_seed(137) | ||
|
|
||
| result = transform(image=image)["image"] | ||
| holes = transform.params["holes"] | ||
|
|
||
| assert holes.shape == (1, 4) | ||
| x_min, y_min, x_max, y_max = holes[0] | ||
|
|
||
| outside_mask = np.ones(image.shape[:2], dtype=bool) | ||
| outside_mask[y_min:y_max, x_min:x_max] = False | ||
|
|
||
| np.testing.assert_array_equal(result[outside_mask], image[outside_mask]) | ||
|
|
||
| hole_patch = result[y_min:y_max, x_min:x_max] | ||
| np.testing.assert_array_equal(hole_patch[..., 0], hole_patch[..., 1]) | ||
| np.testing.assert_array_equal(hole_patch[..., 1], hole_patch[..., 2]) |
There was a problem hiding this comment.
suggestion (testing): Consider asserting that the dtype is preserved when using fill="grayscale".
To better pin down the behavior of this mode, please also assert that result.dtype == image.dtype (here np.uint8), so that any future change that alters the dtype (e.g., to float) will be caught by this test.
Suggested change
| def test_erasing_grayscale_fill_converts_only_sampled_patch(): | |
| image = TestDataFactory.create_image((64, 64, 3), dtype=np.uint8, seed=137) | |
| transform = A.Erasing( | |
| scale=(0.2, 0.2), | |
| ratio=(1.0, 1.0), | |
| fill="grayscale", | |
| p=1.0, | |
| ) | |
| transform.set_random_seed(137) | |
| result = transform(image=image)["image"] | |
| holes = transform.params["holes"] | |
| assert holes.shape == (1, 4) | |
| x_min, y_min, x_max, y_max = holes[0] | |
| outside_mask = np.ones(image.shape[:2], dtype=bool) | |
| outside_mask[y_min:y_max, x_min:x_max] = False | |
| np.testing.assert_array_equal(result[outside_mask], image[outside_mask]) | |
| hole_patch = result[y_min:y_max, x_min:x_max] | |
| np.testing.assert_array_equal(hole_patch[..., 0], hole_patch[..., 1]) | |
| np.testing.assert_array_equal(hole_patch[..., 1], hole_patch[..., 2]) | |
| def test_erasing_grayscale_fill_converts_only_sampled_patch(): | |
| image = TestDataFactory.create_image((64, 64, 3), dtype=np.uint8, seed=137) | |
| transform = A.Erasing( | |
| scale=(0.2, 0.2), | |
| ratio=(1.0, 1.0), | |
| fill="grayscale", | |
| p=1.0, | |
| ) | |
| transform.set_random_seed(137) | |
| result = transform(image=image)["image"] | |
| holes = transform.params["holes"] | |
| # Ensure that using fill="grayscale" does not change the dtype of the image | |
| assert result.dtype == image.dtype | |
| assert holes.shape == (1, 4) | |
| x_min, y_min, x_max, y_max = holes[0] | |
| outside_mask = np.ones(image.shape[:2], dtype=bool) | |
| outside_mask[y_min:y_max, x_min:x_max] = False | |
| np.testing.assert_array_equal(result[outside_mask], image[outside_mask]) | |
| hole_patch = result[y_min:y_max, x_min:x_max] | |
| np.testing.assert_array_equal(hole_patch[..., 0], hole_patch[..., 1]) | |
| np.testing.assert_array_equal(hole_patch[..., 1], hole_patch[..., 2]) |
Comment on lines
+1341
to
+1367
| def test_erasing_grayscale_fill_on_volume(): | ||
| volume = TestDataFactory.create_volume((4, 64, 64, 3), dtype=np.uint8, seed=137) | ||
|
|
||
| transform = A.Erasing( | ||
| scale=(0.2, 0.2), | ||
| ratio=(1.0, 1.0), | ||
| fill="grayscale", | ||
| p=1.0, | ||
| ) | ||
| transform.set_random_seed(137) | ||
|
|
||
| result = transform(volume=volume)["volume"] | ||
| holes = transform.params["holes"] | ||
|
|
||
| assert holes.shape == (1, 4) | ||
| x_min, y_min, x_max, y_max = holes[0] | ||
|
|
||
| result_outside = result.copy() | ||
| volume_outside = volume.copy() | ||
| result_outside[:, y_min:y_max, x_min:x_max, :] = 0 | ||
| volume_outside[:, y_min:y_max, x_min:x_max, :] = 0 | ||
|
|
||
| np.testing.assert_array_equal(result_outside, volume_outside) | ||
|
|
||
| hole_patch = result[:, y_min:y_max, x_min:x_max, :] | ||
| np.testing.assert_array_equal(hole_patch[..., 0], hole_patch[..., 1]) | ||
| np.testing.assert_array_equal(hole_patch[..., 1], hole_patch[..., 2]) |
There was a problem hiding this comment.
suggestion (testing): Strengthen the volume test by verifying that the erased region actually changes relative to the input.
Currently the test only confirms that the outside region is unchanged and that the erased region is grayscale, but not that the grayscale values differ from the original volume. To guard against regressions where the grayscale fill becomes a no-op, add an assertion like assert not np.array_equal(hole_patch, volume[:, y_min:y_max, x_min:x_max, :]) (or a similar pixel-level check) to ensure the erased patch is actually modified.
Suggested change
| def test_erasing_grayscale_fill_on_volume(): | |
| volume = TestDataFactory.create_volume((4, 64, 64, 3), dtype=np.uint8, seed=137) | |
| transform = A.Erasing( | |
| scale=(0.2, 0.2), | |
| ratio=(1.0, 1.0), | |
| fill="grayscale", | |
| p=1.0, | |
| ) | |
| transform.set_random_seed(137) | |
| result = transform(volume=volume)["volume"] | |
| holes = transform.params["holes"] | |
| assert holes.shape == (1, 4) | |
| x_min, y_min, x_max, y_max = holes[0] | |
| result_outside = result.copy() | |
| volume_outside = volume.copy() | |
| result_outside[:, y_min:y_max, x_min:x_max, :] = 0 | |
| volume_outside[:, y_min:y_max, x_min:x_max, :] = 0 | |
| np.testing.assert_array_equal(result_outside, volume_outside) | |
| hole_patch = result[:, y_min:y_max, x_min:x_max, :] | |
| np.testing.assert_array_equal(hole_patch[..., 0], hole_patch[..., 1]) | |
| np.testing.assert_array_equal(hole_patch[..., 1], hole_patch[..., 2]) | |
| def test_erasing_grayscale_fill_on_volume(): | |
| volume = TestDataFactory.create_volume((4, 64, 64, 3), dtype=np.uint8, seed=137) | |
| transform = A.Erasing( | |
| scale=(0.2, 0.2), | |
| ratio=(1.0, 1.0), | |
| fill="grayscale", | |
| p=1.0, | |
| ) | |
| transform.set_random_seed(137) | |
| result = transform(volume=volume)["volume"] | |
| holes = transform.params["holes"] | |
| assert holes.shape == (1, 4) | |
| x_min, y_min, x_max, y_max = holes[0] | |
| result_outside = result.copy() | |
| volume_outside = volume.copy() | |
| result_outside[:, y_min:y_max, x_min:x_max, :] = 0 | |
| volume_outside[:, y_min:y_max, x_min:x_max, :] = 0 | |
| np.testing.assert_array_equal(result_outside, volume_outside) | |
| hole_patch = result[:, y_min:y_max, x_min:x_max, :] | |
| volume_hole_patch = volume[:, y_min:y_max, x_min:x_max, :] | |
| # Ensure the erased region actually changes relative to the original volume | |
| assert not np.array_equal(hole_patch, volume_hole_patch) | |
| np.testing.assert_array_equal(hole_patch[..., 0], hole_patch[..., 1]) | |
| np.testing.assert_array_equal(hole_patch[..., 1], hole_patch[..., 2]) |
There was a problem hiding this comment.
Pull request overview
This PR adds a new fill="grayscale" mode for dropout-style cutout operations (notably A.Erasing), allowing erased regions to be converted to grayscale instead of being replaced by a constant/random value or inpainted.
Changes:
- Extend
BaseDropout/Erasingfillto support"grayscale"and add related validation. - Implement grayscale hole filling in
albumentations.augmentations.dropout.functionalfor images, volumes, and batches. - Add test coverage for grayscale fill on images and (batched) volumes, plus validation/error cases.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
tests/test_augmentations.py |
Adds unit tests validating grayscale fill behavior and constraints. |
albumentations/augmentations/dropout/transforms.py |
Extends fill schema/type union and adds grayscale-specific validation for channel count and fill_mask. |
albumentations/augmentations/dropout/functional.py |
Adds grayscale hole filling implementations and integrates them into cutout* functions. |
albumentations/augmentations/dropout/coarse_dropout.py |
Updates Erasing docs/types and minor return-shape cleanup in parameter sampling. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
330
to
332
| if max_valid_area < min_area: | ||
| return {"holes": np.array([], dtype=np.int32).reshape((0, 4))} | ||
| return {"holes": np.empty((0, 4), dtype=np.int32)} | ||
|
|
ternaus
reviewed
May 14, 2026
| return img | ||
|
|
||
| if num_channels not in {1, 3}: | ||
| raise ValueError("Grayscale fill works only for 1 or 3 channel images") |
Contributor
There was a problem hiding this comment.
There are many ways to convert to grayscale, most of them support any number of channels, no need to use to_gray_weighted_average
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Add a grayscale fill mode to erasing/cutout dropout transforms for images, volumes, and volume batches, with appropriate validation and support across the dropout pipeline.
New Features:
Enhancements:
Tests: