-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Fix solarize #1991
Fix solarize #1991
Conversation
Reviewer's Guide by SourceryThis pull request fixes an issue with the Solarize augmentation and adds new tests for the label function. The main changes include modifying the Solarize threshold type, updating dependencies, and adding comprehensive tests for the label function. Updated class diagram for Solarize augmentationclassDiagram
class Solarize {
+ZeroPlusRangeType threshold
+__init__(threshold: ScaleFloatType = (128, 128), p: float = 0.5, always_apply: bool | None = None)
}
note for Solarize "Threshold type changed from OnePlusFloatRangeType to ZeroPlusRangeType"
Class diagram for new ZeroPlusRangeTypeclassDiagram
class ZeroPlusRangeType {
+convert_to_0plus_range()
+nondecreasing()
}
note for ZeroPlusRangeType "New type added with validators convert_to_0plus_range and nondecreasing"
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:
- The PR includes changes unrelated to the 'solarize' fix (new test_dropout.py file and pre-commit hook updates). Consider splitting these into separate PRs for better focus and easier review.
- The 'solarize' fix itself looks good, with appropriate test cases added. However, the PR title and description don't reflect the full scope of changes included.
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟡 Testing: 1 issue found
- 🟢 Complexity: all looks good
- 🟢 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.
Fixes #1990
Summary by Sourcery
Fix the Solarize transformation to handle threshold values correctly and add tests to verify its behavior. Enhance the annotation system with a new ZeroPlusRangeType. Update pre-commit hooks for pyproject-fmt and mypy. Add functional tests for the label function in dropout augmentations to ensure consistency with scikit-image.
Bug Fixes:
Enhancements:
Build:
Tests: