-
Notifications
You must be signed in to change notification settings - Fork 207
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
Add option to show single progress bar for all workers #203
base: master
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## master #203 +/- ##
==========================================
- Coverage 91.47% 90.30% -1.18%
==========================================
Files 12 12
Lines 575 588 +13
==========================================
+ Hits 526 531 +5
- Misses 49 57 +8
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Hi @ApolloBian, thanks for your contribution! Something that I would like to see before merging is tests. As far as I'm concerned, this can just be turning the feature off and on, as we do with progress bars in general. @nalepae as a sidenote, I think it would be good to enable workflows to run automatically/without approval from maintainers. I don't see any possibilities of the current workflow being used maliciously, but it would speed up development. |
@@ -205,6 +206,7 @@ def parallelize_with_memory_file_system( | |||
nb_requested_workers: int, | |||
data_type: Type[DataType], | |||
progress_bars_type: ProgressBarsType, | |||
single_bar |
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.
I think it would be good to have type hints for function arguments.
It's the most permissive I can do :) |
any chance this is gonna be merged soon? |
Hey @lpuglia, after the requested changes are implemented this would principally be ready for merge. |
Hey @till-m @ApolloBian @nalepae @lpuglia, is there anything missing for this to be merged? I can help if that's the case. |
Hey @victormaricato, there's a few PR's attempting to tackle this problem but none of them are currently ready to merge: (this), #239 and #243. |
Pandaral·lel is looking for a maintainer! |
As of now, each worker has its own progress bar, but it would be nice to have a summary bar for all workers, since sometimes it will be too cluttered to show all progress bars when the number of workers are too high, as mentioned in #134
I implemented this feature to allow users to pass an argument to
pandarallel.initialize
to select to use only one progress bar. The test results are shown below: