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

Cannot import on python3: missing c_distances_openmp #1157

Closed
jbarnoud opened this issue Jan 12, 2017 · 7 comments · Fixed by #1158
Closed

Cannot import on python3: missing c_distances_openmp #1157

jbarnoud opened this issue Jan 12, 2017 · 7 comments · Fixed by #1158
Assignees

Comments

@jbarnoud
Copy link
Contributor

This issue got mentioned first in #929.

Expected behaviour

MDAnalysis get imported on python 3.

Actual behaviour

>>> import MDAnalysis
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/jon/Envs/mda3/lib/python3.4/site-packages/MDAnalysis/__init__.py", line 178, in <module>
    from .lib import log
  File "/home/jon/Envs/mda3/lib/python3.4/site-packages/MDAnalysis/lib/__init__.py", line 36, in <module>
    from . import distances  # distances relies on mdamath
  File "/home/jon/Envs/mda3/lib/python3.4/site-packages/MDAnalysis/lib/distances.py", line 121, in <module>
    from c_distances_openmp import OPENMP_ENABLED as USED_OPENMP
ImportError: No module named 'c_distances_openmp'

This seems to come from the name given to the so files:

# Python 2.7
$ ls /home/jon/Envs/mda2/lib/python2.7/site-packages/MDAnalysis/lib
c_distances_openmp.so  formats       log.pyc            NeighborSearch.pyc   _transformations.so
c_distances.so         __init__.py   mdamath.py         qcprot.so            util.py
distances.py           __init__.pyc  mdamath.pyc        transformations.py   util.pyc
distances.pyc          log.py        NeighborSearch.py  transformations.pyc

# Python 3.4
$ ls /home/jon/Envs/mda3/lib/python3.4/site-packages/MDAnalysis/lib
c_distances.cpython-34m.so         formats      mdamath.py         qcprot.cpython-34m.so            util.py
c_distances_openmp.cpython-34m.so  __init__.py  NeighborSearch.py  _transformations.cpython-34m.so
distances.py                       log.py       __pycache__        transformations.py

Deleting the from c_distances_openmp import OPENMP_ENABLED as USED_OPENMP seems to fix the error (at least at import time). Yet, I expect the USED_OPENMP variable to be used somewhere.

Code to reproduce the behaviour

On python 3:

import MDAnalysis

Currently version of MDAnalysis:

dev

@richardjgowers
Copy link
Member

As a fix to the fix, there's a try/except block before this that tries to import the openmp module. The USED_OPENMP import should be moved into this block and set in the except block to False

@kain88-de
Copy link
Member

This is still weird. This shouldn't require a try/except block

@jbarnoud can you import the c_distances_openmp module under python 3 if it's build?

@jbarnoud
Copy link
Contributor Author

@kain88-de No I can't. But if I rename c_distances.cpython-34m.so to c_distances_openmp.so then it works.

I do not know why the cython files are named this way in python 3 now. I have the issue with cython 0.23.4, I'll try to upgrade cython and see if it changes anything.

@kain88-de
Copy link
Member

Does this so file name mangling also happen when you create a small test cython project?

@jbarnoud
Copy link
Contributor Author

After some more fiddling, it appears that the name mangling comes from PEP3149 and has no impact on whether or not you can import the module. What was causing the problem was that the import was implicitly relative:

 from c_distances_openmp import OPENMP_ENABLED as USED_OPENMP

instead of

from .c_distances_openmp import OPENMP_ENABLED as USED_OPENMP

I'll push the fix latter today or tomorrow with other python 3 fixes.

@kain88-de
Copy link
Member

ah yeah that looks like a proper fix. Thanks for looking into it.

@richardjgowers
Copy link
Member

@jbarnoud good spot! Make sure you push onto the right branch, we seem to have 2 currently (#929 and #1155), I think the latter is just improvements on the first, so maybe we could close #929 and work from #1155 instead? ( @tylerjereddy @kain88-de ?)

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 a pull request may close this issue.

3 participants