-
Notifications
You must be signed in to change notification settings - Fork 47
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
Refactor progress bars #1272
Refactor progress bars #1272
Conversation
tqdm progress bars are used in a couple of places. They don't play so well in non-interactive jobs, testing, ... . They also cause trouble with nbspinx (ICB-DCM#1246, ICB-DCM#1271). Progress bars can be disabled for specific tasks, but not globally (or at least not very conveniently). Since recently, tqdm can be controlled via environment variables (e.g., disabling all progress bars or changing update frequency). However, this works by changing the argument defaults, so it only works if we don't pass explicit `disable=...`. Therefore, this PR introduces some wrapper that checks whether the user explicitly enabled/disabled progress bars. If not, we go with the tqdm default, which means showing all progress bars unless globally disabled. An additional `enabled` argument is added for convenience.
Codecov ReportAttention:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## develop #1272 +/- ##
===========================================
- Coverage 84.60% 84.59% -0.02%
===========================================
Files 148 148
Lines 12112 12122 +10
===========================================
+ Hits 10247 10254 +7
- Misses 1865 1868 +3 ☔ View full report in Codecov by Sentry. |
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.
👍
Co-authored-by: Dilan Pathirana <[email protected]>
A subset of ICB-DCM#1246. After ICB-DCM#1272. Co-authored-by: Doresic <[email protected]>
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 to me.
A subset of ICB-DCM#1246. After ICB-DCM#1272. Co-authored-by: Doresic <[email protected]>
tqdm progress bars are used in a couple of places. They don't play so well in non-interactive jobs, testing, ... . They also cause trouble with nbspinx (#1246, #1271).
Progress bars can be disabled for specific tasks, but not globally (or at least not very conveniently).
Since recently, tqdm can be controlled via environment variables (e.g., disabling all progress bars or changing update frequency). However, this works by changing the argument defaults, so it only works if we don't pass explicit
disable=...
. Therefore, this PR introduces some wrapper that checks whether the user explicitly enabled/disabled progress bars. If not, we go with the tqdm default, which means showing all progress bars unless globally disabled. An additionalenabled
argument is added for convenience.