-
Notifications
You must be signed in to change notification settings - Fork 429
FEA parallel_config context manager to allow more fine-grained control #1392
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
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #1392 +/- ##
==========================================
- Coverage 94.78% 94.75% -0.03%
==========================================
Files 44 44
Lines 7207 7306 +99
==========================================
+ Hits 6831 6923 +92
- Misses 376 383 +7
... and 1 file with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ 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.
From a quick scan, the setup looks nice :)
Just a few first comments.
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.
To me, this is a valuable contribution for users. Thank you, @jeremiedbb.
Here is a review from an outsider (I am starting getting around joblib).
@jeremiedbb There are a few conflicts to resolve after the merge of the |
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 did a pass but the code looks super nice!! Good job @jeremiedbb :)
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! thanks @jeremiedbb for making this :)
Thanks a lot for this feature. |
Fixes #912
Fixes #979
related to #1376
adds a new context_manager
parallel_config
that should be used in place ofparallel_backend
. It exposes more parameters and does not enforce setting a backend.It allows to do something like:
It's using the threading backend instead of the default loky, with 3 jobs instead of the default 1.
The config parameters of the context are used only if they are not explicitely set in
Parallel
. It makes us forced to use sentinels for the default values, adding a little bit of complexity.TODO