Add transforms #2148
Add transforms #2148
Conversation
Reviewer's Guide by SourceryThis PR introduces three new image transformation classes (RandomPosterize, RandomSaturation, GaussianNoise) and enhances input validation for tuple bounds. The changes focus on improving API compatibility with other libraries like Kornia and torchvision while maintaining backward compatibility through aliases and parameter validation. Class diagram for new and updated transformsclassDiagram
class Posterize {
<<interface>>
}
class RandomPosterize {
num_bits: tuple[int, int] = (3, 3)
p: float = 0.5
get_transform_init_args_names() tuple[str, ...]
}
Posterize <|-- RandomPosterize
note for RandomPosterize "Alias for Posterize for Kornia API compatibility"
class ColorJitter {
<<interface>>
}
class RandomSaturation {
saturation: tuple[float, float] = (1.0, 1.0)
p: float = 0.5
get_transform_init_args_names() tuple[str]
}
ColorJitter <|-- RandomSaturation
note for RandomSaturation "Specialized version of ColorJitter for saturation"
class GaussNoise {
<<interface>>
}
class GaussianNoise {
mean: float = 0.0
sigma: float = 0.1
p: float = 0.5
get_transform_init_args_names() tuple[str, ...]
}
GaussNoise <|-- GaussianNoise
note for GaussianNoise "Specialized version of GaussNoise following torchvision's API"
File-Level Changes
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 @ternaus - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2148 +/- ##
=========================================
+ Coverage 0 90.39% +90.39%
=========================================
Files 0 48 +48
Lines 0 8143 +8143
=========================================
+ Hits 0 7361 +7361
- Misses 0 782 +782 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Hey @ternaus - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟡 Testing: 2 issues found
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| hue=(0.0, 0.0), # No hue change | ||
| p=p, | ||
| ) | ||
| self.saturation = saturation |
There was a problem hiding this comment.
suggestion: Remove redundant saturation attribute storage as it's already stored in parent class
The saturation parameter is being stored both in the parent ColorJitter class and in RandomSaturation. This creates unnecessary duplication.
| sigma = self.py_random.uniform(*self.std_range) | ||
|
|
||
| sigma *= max_value | ||
| mean = self.py_random.uniform(*self.mean_range) * max_value | ||
|
|
||
| if self.per_channel: | ||
| target_shape = image.shape | ||
| if self.noise_scale_factor == 1: |
There was a problem hiding this comment.
suggestion: Extract duplicated noise generation logic into a helper method
The noise generation code is duplicated between per_channel and non-per_channel branches. A helper method would reduce duplication and improve maintainability.
def _generate_noise(self, target_shape, mean, sigma):
if self.noise_scale_factor == 1:
return self.random_generator.normal(mean, sigma, target_shape)
return self.random_generator.normal(mean, sigma, target_shape) * self.noise_scale_factor| data={"image": image}, | ||
| ) | ||
|
|
||
| assert np.abs(mean - apply_params["gauss"].mean()) < 0.5 | ||
| assert np.abs(mean - apply_params["gauss"].mean() / MAX_VALUES_BY_DTYPE[image.dtype]) < 0.5 |
There was a problem hiding this comment.
issue (testing): GaussNoise test tolerance seems too high
The test allows for a large deviation (0.5) when checking the mean of the generated noise. Consider tightening this tolerance to ensure more precise noise generation, especially since the mean is now normalized to [0,1] range.
| data={"image": image}, | ||
| ) | ||
|
|
||
| assert np.abs(mean - apply_params["gauss"].mean()) < 0.5 | ||
| assert np.abs(mean - apply_params["gauss"].mean() / MAX_VALUES_BY_DTYPE[image.dtype]) < 0.5 | ||
| result = A.Compose([aug], seed=42)(image=image) | ||
|
|
||
| assert not (result["image"] >= image).all() |
There was a problem hiding this comment.
suggestion (testing): Missing test cases for GaussianNoise std_range parameter
The test only verifies the mean parameter. Add test cases to verify that the standard deviation (sigma) of the generated noise matches the specified std_range parameter.
def test_gauss_noise(mean, image, std=0.1):
aug = A.GaussNoise(p=1, noise_scale_factor=1.0, mean_range=(mean, mean), std_range=(std, std))
aug.set_random_seed(42)
apply_params = aug.get_params_dependent_on_data(data={"image": image})
assert np.abs(mean - apply_params["gauss"].mean() / MAX_VALUES_BY_DTYPE[image.dtype]) < 0.5
assert np.abs(std - apply_params["gauss"].std() / MAX_VALUES_BY_DTYPE[image.dtype]) < 0.1
result = A.Compose([aug], seed=42)(image=image)
assert not (result["image"] >= image).all()| self.per_channel = per_channel | ||
| self.noise_scale_factor = noise_scale_factor | ||
|
|
||
| self.var_limit = var_limit | ||
|
|
||
| def apply(self, img: np.ndarray, gauss: np.ndarray, **params: Any) -> np.ndarray: | ||
| return fmain.add_noise(img, gauss) | ||
|
|
||
| def get_params_dependent_on_data(self, params: dict[str, Any], data: dict[str, Any]) -> dict[str, float]: |
There was a problem hiding this comment.
issue (complexity): Consider extracting noise generation logic into a helper method to reduce code duplication.
The noise generation logic has unnecessary duplication between the per_channel branches. Consider extracting the common logic into a helper method:
def _generate_noise(self, target_shape: tuple, mean: float, sigma: float) -> np.ndarray:
if self.noise_scale_factor == 1:
return self.random_generator.normal(mean, sigma, target_shape)
return fmain.generate_approx_gaussian_noise(
target_shape,
mean,
sigma,
self.noise_scale_factor,
self.random_generator,
)
def get_params_dependent_on_data(self, params: dict[str, Any], data: dict[str, Any]) -> dict[str, float]:
image = data["image"] if "image" in data else data["images"][0]
max_value = MAX_VALUES_BY_DTYPE[image.dtype]
sigma = self.py_random.uniform(*self.std_range) * max_value
mean = self.py_random.uniform(*self.mean_range) * max_value
if self.per_channel:
gauss = self._generate_noise(image.shape, mean, sigma)
else:
gauss = self._generate_noise(image.shape[:2], mean, sigma)
if image.ndim > MONO_CHANNEL_DIMENSIONS:
gauss = np.expand_dims(gauss, -1)
return {"gauss": gauss}This refactoring:
- Eliminates code duplication in noise generation
- Improves readability by separating noise generation from shape handling
- Makes the logic flow clearer and easier to maintain
| if self.mean is not None: | ||
| self.mean_range = (0.0, 0.0) | ||
|
|
||
| if self.mean is not None: |
There was a problem hiding this comment.
suggestion (code-quality): Merge repeated if statements into single if (merge-repeated-ifs)
| if self.mean is not None: |
|
|
||
| def validator(value: tuple[Number, Number]) -> tuple[Number, Number]: | ||
| if max_val is None: | ||
| if not (value[0] >= min_val and value[1] >= min_val): |
There was a problem hiding this comment.
suggestion (code-quality): Simplify logical expression using De Morgan identities (de-morgan)
| if not (value[0] >= min_val and value[1] >= min_val): | |
| if value[0] < min_val or value[1] < min_val: |
Addresses:
#2106
#2109
#2094
Summary by Sourcery
Add the RandomPosterize transform as an alias for Posterize to support Kornia API compatibility, and introduce input validation for tuple bounds.
New Features:
Enhancements:
Tests:
Summary by Sourcery
Introduce new transforms RandomPosterize, RandomSaturation, and GaussianNoise to enhance image augmentation capabilities and ensure compatibility with Kornia and torchvision APIs. Improve input validation with a new range bounds checker and update documentation to reflect these changes.
New Features:
Enhancements:
Documentation:
Tests: