-
-
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
Fix padding #1856
Fix padding #1856
Conversation
Reviewer's Guide by SourceryThis pull request refactors the elastic transformation functions, updates the ElasticTransform class to remove the alpha_affine parameter, and improves padding handling. It also updates tests and serialization files accordingly, and adds new tests for padding color. Additionally, it refactors parameter handling in crop and pad functions and updates valid interpolation and border modes. File-Level Changes
Tips
|
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:
- Consider adding unit tests for the new
elastic_transform_precise
andelastic_transform_approximate
functions to ensure they behave as expected. - The removal of
alpha_affine
parameter might be a breaking change. Consider adding a deprecation warning if this parameter is provided, to ease the transition for existing users.
Here's what I looked at during the review
- 🟡 General issues: 6 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
Fixes #1853
Summary by Sourcery
This pull request addresses padding issues and refactors the elastic transformation functionality. It introduces new functions for precise and approximate elastic transformations, improves the handling of border values, and updates the
ElasticTransform
class to remove deprecated parameters. Additionally, new tests are added to ensure the correctness of these changes.elastic_transform_precise
andelastic_transform_approximate
functions for precise and approximate elastic transformations respectively.warp_affine_with_value_extension
andcopy_make_border_with_value_extension
functions to handle extended border values.elastic_transform
function to delegate to eitherelastic_transform_precise
orelastic_transform_approximate
based on theapproximate
parameter.ElasticTransform
class to remove thealpha_affine
parameter and deprecated its usage, suggesting the use of theAffine
transform instead.extend_value
function to ensure correct padding for multi-channel images.ElasticTransform
class to provide clearer documentation and parameter descriptions.elastic_transform
function and the removal of thealpha_affine
parameter.