Thanks for detecting this and the issue at joblib! Let's see if there is a way to fix it on the joblib side (or maybe our config mechanism needs to be revisited). Using a parallel wrapper means that external users of joblib + scikit-learn would still run into this issue..

@rth do you have a proposal for a revised config handling?

I have a "fun" proposal here: joblib/joblib#1071 (comment)

(Global config + doing things in parallel = fun)

We could just use os.environ to store the config?

Storing them in the environment may not work: joblib/joblib#1064

Doesn't that depend on the backend? It should work as long as all those processes are on the same machine, doesn't it?

Maybe we could pass it as environment and make joblib / loky detect that environment has changed and re-spawn workers accordingly. That might be a bit expensive though.

Doesn't that depend on the backend? It should work as long as all those processes are on the same machine, doesn't it?

The threading backend should have no problem. multiprocessing + fork should work out of the box but it's broken in other ways and not available on windows.

loky and dask will need explicit config init.

I think I'm okay with this, but I'd probably move the code to fixes.py to have them all in the same place, and I think adding a linter to fail on importing joblib.delayed would be an overkill?

Apart from not being comfortable with the use of utils.fixes for something other than a backport, this looks great

Should we use sklearn.utls._parallel like we had before?

This looks reasonable but would one among @lesteve @ogrisel or @tomMoral be also be able to review this, following discussion in joblib/joblib#1071?

Could you please also check that the vendored version of delayed, hasn't significantly changed for the supported versions of joblib?

I looked at:

https://github.com/joblib/joblib/blob/19db4f58ac502a4bd01ab07aa31b2fc8756bcc3a/joblib/parallel.py#L302

from tags 0.11 to 0.16 and the function does the same thing.

After reading its implementation, we can use a simplified version in our utils.fixes for now:

def delayed(function):
    """Decorator used to capture the arguments of a function."""
    @functools.wraps(function)
    def delayed_function(*args, **kwargs):
        return _FuncWrapper(function), args, kwargs
    return delayed_function

This way we do not need to depend on a vendored version.

Hi @scikit-learn/core-devs, two approvals here: is something left? Or is it good to merge?

Since the last commit and hence the CI runs were from almost 3 weeks ago, I'm happy to merge once we do another merge with the current master. Would you mind updating @thomasjpfan ? I feel like we agreed to having this in the meantime.

Merged with master and it still passes.

woohoo 🥳 let's do this then 👍