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

Don't try to import a toplevel __init__ module #131

Merged
merged 2 commits into from
Feb 15, 2023

Conversation

smurfix
Copy link
Contributor

@smurfix smurfix commented Feb 13, 2023

I have no idea on which Python versions the old code works, but on mine (Python 3.11, Debian) it doesn't.

@mmckerns
Copy link
Member

Can you clarify what issue you are reporting/patching? It's best if you provide a description of the expected vs actual behavior of a sample code that reproduces the behavior. The tests run as expected on my test machines and also on the CI server... so I'm not sure what you are trying to do.

@smurfix
Copy link
Contributor Author

smurfix commented Feb 14, 2023

Well, errors like this one. I'm packaging the module for Debian, and this is from the Debian build process which basically runs python3.11 setup.py build and then proceeds to testing:

cd .pybuild/cpython3_3.11_multiprocess/build; python3.11 -m unittest discover -v 
…
ImportError: Failed to import test module: multiprocess.tests.test_multiprocessing_spawn
Traceback (most recent call last):
  File "/usr/lib/python3.11/unittest/loader.py", line 407, in _find_test_path
    module = self._get_module_from_name(name)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/unittest/loader.py", line 350, in _get_module_from_name
    __import__(name)
  File "/src/multiprocess/.pybuild/cpython3_3.11_multiprocess/build/multiprocess/tests/test_multiprocessing_spawn.py", line 2, in <module>
    import __init__ as _test_multiprocessing
ModuleNotFoundError: No module named '__init__'

or (second patch)

Traceback (most recent call last):
  File "/src/multiprocess/.pybuild/cpython3_3.11_multiprocess/build/multiprocess/tests/mp_preload.py", line 14, in <module>
    __import__(modname)
ModuleNotFoundError: No module named 'test.mp_preload'

Basically, your Travis script runs the tests directly, which has the side effect of adding the directory the test script is in to PYTHONPATH. This doesn't happen when you allow unittest to discover the tests, thus the above test run fails.

Replacing the imports with fully-qualified module names (relative ones won't work when you run a test directly) doesn't (IMHO cannot) hurt, Just Works, and allows me to package your module for Debian without adding some multiprocess-specific PYTHONPATH setup to the "test" step.

@mmckerns
Copy link
Member

mmckerns commented Feb 15, 2023

Sometimes I'm seeing the same error as you, and sometimes I'm not (it must depend on the directory you run the command from)... however, if I do (run python setup.py build) then cd into build/lib... then I can reproduce the same error as you are reporting. We shouldn't be supporting python setup,py build invocations, however... the same error happens if you use python -m build -s and then unpack the source distribution and then run from certain directories inside it. So... ok. If this is an issue... and there's no downside, then sure. Let me see if there was a reason the tests were imported using __init__... um, I don't think so. Yeah, I'm pretty sure what you have is better. I'll do some testing then resolve this. Thanks.

@mmckerns mmckerns added this to the multiprocess-0.70.15 milestone Feb 15, 2023
Copy link
Member

@mmckerns mmckerns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mmckerns mmckerns merged commit 125b355 into uqfoundation:master Feb 15, 2023
@smurfix smurfix deleted the import branch February 16, 2023 21:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants