Skip to content
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 test for missing _multiprocessing module #1246

Closed
wants to merge 10 commits into from

Conversation

hoodmane
Copy link
Contributor

@hoodmane hoodmane commented Jan 10, 2022

Pyodide and other single-threaded Python builds will be missing the _multiprocessing module. Test that joblib still works in this environment.

Test is currently failing
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/home/hood/Documents/programming/joblib/joblib/__init__.py", line 120, in <module>
    from .parallel import Parallel
  File "/home/hood/Documents/programming/joblib/joblib/parallel.py", line 30, in <module>
    from .externals import loky
  File "/home/hood/Documents/programming/joblib/joblib/externals/loky/__init__.py", line 11, in <module>
    from .backend.context import cpu_count
  File "/home/hood/Documents/programming/joblib/joblib/externals/loky/backend/__init__.py", line 4, in <module>
    from .context import get_context
  File "/home/hood/Documents/programming/joblib/joblib/externals/loky/backend/context.py", line 23, in <module>
    from .process import LokyProcess, LokyInitMainProcess
  File "/home/hood/Documents/programming/joblib/joblib/externals/loky/backend/process.py", line 11, in <module>
    from .compat import BaseProcess
  File "/home/hood/Documents/programming/joblib/joblib/externals/loky/backend/compat.py", line 25, in <module>
    from .compat_posix import wait
  File "/home/hood/Documents/programming/joblib/joblib/externals/loky/backend/compat_posix.py", line 13, in <module>
    from multiprocessing.connection import wait
  File "/usr/lib/python3.9/multiprocessing/connection.py", line 21, in <module>
    import _multiprocessing
  File "/home/hood/Documents/programming/joblib/joblib/test/missing_multiprocessing/_multiprocessing.py", line 5, in <module>
    raise ImportError("No _multiprocessing module!")
ImportError: No _multiprocessing module!

This is a blocker for Pyodide to upgrade joblib.

@codecov
Copy link

codecov bot commented Jan 10, 2022

Codecov Report

Merging #1246 (da7fb7f) into master (7742f58) will decrease coverage by 1.14%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1246      +/-   ##
==========================================
- Coverage   88.43%   87.29%   -1.15%     
==========================================
  Files          47       48       +1     
  Lines        7082     7090       +8     
==========================================
- Hits         6263     6189      -74     
- Misses        819      901      +82     
Impacted Files Coverage Δ
joblib/test/test_missing_multiprocessing.py 100.00% <100.00%> (ø)
joblib/test/test_hashing.py 89.86% <0.00%> (-9.22%) ⬇️
joblib/test/test_memmapping.py 91.44% <0.00%> (-7.80%) ⬇️
joblib/_multiprocessing_helpers.py 76.00% <0.00%> (-4.00%) ⬇️
joblib/test/test_parallel.py 93.95% <0.00%> (-1.82%) ⬇️
joblib/numpy_pickle_utils.py 92.13% <0.00%> (-1.13%) ⬇️
joblib/compressor.py 88.15% <0.00%> (-0.99%) ⬇️
joblib/test/test_numpy_pickle.py 92.69% <0.00%> (-0.64%) ⬇️
joblib/parallel.py 96.14% <0.00%> (-0.28%) ⬇️
joblib/_parallel_backends.py 93.38% <0.00%> (+1.47%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7742f58...da7fb7f. Read the comment docs.

@hoodmane
Copy link
Contributor Author

I'm a bit confused why Testing linux_py36_no_multiprocessing_no_lzma doesn't already cover this.

@rth
Copy link
Contributor

rth commented Jan 10, 2022

Pyodide and other single-threaded Python builds will be missing the _multiprocessing module. Test that joblib still works in this environment.

To give more context, CPython can be built without multiprocessing, in which case multiprocessing module will import fine, but _multiprocessing and likely some of the multiprocessing's submodules fail to import. Such checks are for instance done in the stlib to conditionally skip tests.

This MR generally is part of the effort to update the version of joblib used in Pyodide. So far we have been using 0.11 because it's the last version before the loky addition, which has a bit more multiprocessing imports.

@hoodmane
Copy link
Contributor Author

I think joblib/loky#309 fixes these tests.

@GaelVaroquaux
Copy link
Member

Thank you for the PR pointing out the issue.

We cannot merge a PR with failing tests.

We probably need to wait for joblib/loky#309

@GaelVaroquaux
Copy link
Member

With a closer look, I wonder if it is not better to try to put all the guards in joblib and not loky: loky makes no sense without multiprocessing. I'm giving it a quick try right.

@GaelVaroquaux GaelVaroquaux mentioned this pull request Feb 3, 2022
@GaelVaroquaux
Copy link
Member

I am making an attempt to complete this in #1256

@ogrisel
Copy link
Contributor

ogrisel commented Feb 4, 2022

Closing in favor of #1256.

@ogrisel ogrisel closed this Feb 4, 2022
@ogrisel
Copy link
Contributor

ogrisel commented Feb 4, 2022

Thanks for the contribution @hoodmane !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants