Skip to content
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 parameter assignment for AdvancedBlur #1593

Merged
merged 2 commits into from
Mar 18, 2024
Merged

Fix parameter assignment for AdvancedBlur #1593

merged 2 commits into from
Mar 18, 2024

Conversation

Aloqeely
Copy link
Contributor

This PR fixes 2 things:

  1. Allows to actually use the deprecated sigmaX_limit and sigmaY_limit parameters as before they were always overwritten by default value of sigma_x_limit and sigma_y_limit
  2. Stopped triggering a warning for using deprecated sigmaX_limit and sigmaY_limit parameters when they are not actually used

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @Aloqeely - I've reviewed your changes and they look great!

General suggestions:

  • Verify that the change in assignment logic for sigma_x_limit and sigma_y_limit does not unintentionally overwrite user-defined values.
  • Ensure that the introduction of Optional types for deprecated parameters does not lead to unexpected behavior in downstream code.
  • Consider adding more detailed documentation or comments explaining the rationale behind the changes for future maintainability.
Here's what I looked at during the review
  • 🟡 General issues: 3 issues found
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Docstrings: all looks good

Thanks for using Sourcery. We offer it for free for open source projects and would be very grateful if you could help us grow. If you like it, would you consider sharing Sourcery on your favourite social media? ✨

Share Sourcery

Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

@ternaus ternaus merged commit 23444c9 into albumentations-team:main Mar 18, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants