-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Unify parameters #2160
Unify parameters #2160
Conversation
Reviewer's Guide by SourceryThis pull request unifies and standardizes parameter naming across various transforms in the library, particularly focusing on parameters related to border handling and fill values. The main changes involve renaming parameters to be more consistent, deprecating old parameter names while maintaining backward compatibility, and updating the documentation accordingly. Class diagram for updated transform parametersclassDiagram
class BaseDistortion {
-interpolation: int
-mask_interpolation: int
+apply(img: np.ndarray, map_x: np.ndarray, map_y: np.ndarray, **params: Any): np.ndarray
+apply_to_mask(mask: np.ndarray, map_x: np.ndarray, map_y: np.ndarray, **params: Any): np.ndarray
}
class ElasticTransform {
-interpolation: int
-mask_interpolation: int
+apply(img: np.ndarray, map_x: np.ndarray, map_y: np.ndarray, **params: Any): np.ndarray
+apply_to_mask(mask: np.ndarray, map_x: np.ndarray, map_y: np.ndarray, **params: Any): np.ndarray
}
class Perspective {
-border_mode: int
-fill: ColorType
-fill_mask: ColorType
+apply(img: np.ndarray, matrix, max_width, max_height, **params: Any): np.ndarray
+apply_to_mask(mask: np.ndarray, matrix, max_width, max_height, **params: Any): np.ndarray
}
class Affine {
-border_mode: int
-fill: ColorType
-fill_mask: ColorType
+apply(img: np.ndarray, matrix, **params: Any): np.ndarray
+apply_to_mask(mask: np.ndarray, matrix, **params: Any): np.ndarray
}
class Pad {
-border_mode: BorderModeType
-fill: ColorType
-fill_mask: ColorType
+apply(img: np.ndarray, **params: Any): np.ndarray
+apply_to_mask(mask: np.ndarray, **params: Any): np.ndarray
}
class PadIfNeeded {
-border_mode: int
-fill: ColorType
-fill_mask: ColorType
+apply(img: np.ndarray, **params: Any): np.ndarray
+apply_to_mask(mask: np.ndarray, **params: Any): np.ndarray
}
class Rotate {
-border_mode: int
-fill: ColorType
-fill_mask: ColorType
+apply(img: np.ndarray, matrix, **params: Any): np.ndarray
+apply_to_mask(mask: np.ndarray, matrix, **params: Any): np.ndarray
}
class SafeRotate {
-border_mode: int
-fill: ColorType
-fill_mask: ColorType
+apply(img: np.ndarray, matrix, **params: Any): np.ndarray
+apply_to_mask(mask: np.ndarray, matrix, **params: Any): np.ndarray
}
class CropAndPad {
-border_mode: BorderModeType
-fill: ColorType
-fill_mask: ColorType
+apply(img: np.ndarray, crop_params: Sequence[int], pad_params: Sequence[int], **params: Any): np.ndarray
+apply_to_mask(mask: np.ndarray, crop_params: Sequence[int], pad_params: Sequence[int], **params: Any): np.ndarray
}
class XYMasking {
-fill: ColorType
-fill_mask: ColorType
+apply(img: np.ndarray, **params: Any): np.ndarray
+apply_to_mask(mask: np.ndarray, **params: Any): np.ndarray
}
class CoarseDropout {
-fill: ColorType
-fill_mask: ColorType
+apply(img: np.ndarray, **params: Any): np.ndarray
+apply_to_mask(mask: np.ndarray, **params: Any): np.ndarray
}
class Erasing {
-fill: ColorType
-fill_mask: ColorType
+apply(img: np.ndarray, **params: Any): np.ndarray
+apply_to_mask(mask: np.ndarray, **params: Any): np.ndarray
}
class MaskDropout {
-fill: float | Literal["inpaint"]
-fill_mask: float
+apply(img: np.ndarray, dropout_mask: np.ndarray | None, **params: Any): np.ndarray
+apply_to_mask(mask: np.ndarray, dropout_mask: np.ndarray | None, **params: Any): np.ndarray
}
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 @ternaus - I've reviewed your changes - here's some feedback:
Overall Comments:
- Please document the transforms that were removed (like RandomRotation, RandomHorizontalFlip etc.) in the PR description, explaining the rationale for their removal. This will help users understand and adapt to the breaking changes.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟡 Testing: 2 issues found
- 🟡 Complexity: 2 issues 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 #2160 +/- ##
=========================================
+ Coverage 0 89.72% +89.72%
=========================================
Files 0 47 +47
Lines 0 8469 +8469
=========================================
+ Hits 0 7599 +7599
- Misses 0 870 +870 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
Summary by Sourcery
Unify parameter names across transformation classes, deprecating old names and introducing new ones for consistency. Update documentation and tests to reflect these changes.
Enhancements:
Documentation:
Tests: