-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Avoid deprecated progress_bar_refresh_rate
usage
#10520
Conversation
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.
LGTM !
Hmm, are we sure this is the desired behavior here? If a user explicitly overrides the default and sets |
The user sends a mixed signal, both passing it as a callback and disabling it, we can go for either of them. |
I agree with @ananthsub, I think we should just raise a MisConfigException |
Codecov Report
@@ Coverage Diff @@
## master #10520 +/- ##
=======================================
Coverage 92% 93%
=======================================
Files 179 178 -1
Lines 16270 16190 -80
=======================================
- Hits 15039 14982 -57
+ Misses 1231 1208 -23 |
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.
Looks good!
Co-authored-by: Danielle Pintz <[email protected]>
Co-authored-by: Danielle Pintz <[email protected]>
What does this PR do?
Part of #9939
Does your PR introduce any breaking changes? If yes, please list them.
None
Before submitting
PR review
cc @awaelchli