Skip to content

fix PYTHONPATH for OpenBabel Python bindings#1219

Merged
boegel merged 4 commits intoeasybuilders:developfrom
ostueker:openbabel
Dec 17, 2017
Merged

fix PYTHONPATH for OpenBabel Python bindings#1219
boegel merged 4 commits intoeasybuilders:developfrom
ostueker:openbabel

Conversation

@ostueker
Copy link
Copy Markdown
Contributor

From OpenBabel 2.4.0 on, the Python bindings are no longer installed directly in ${PREFIX}/lib but rather under ${PREFIX}/lib/python2.7/site-packages (for 2.7 Python).

This commit sets the correct PYTHONPATH in the generated module.

ostueker added 3 commits July 13, 2017 12:16
From OpenBabel 2.4.0 on the Python bindings are no longer
installed directly in `${PREFIX}/lib` but rather under
`${PREFIX}/lib/python2.7/site-packages` (for 2.7 Python).

This commit sets the correct PYTHONPATH in the module.
Comment thread easybuild/easyblocks/o/openbabel.py Outdated
# and the Department of Economy, Science and Innovation (EWI) (http://www.ewi-vlaanderen.be/en).
#
# https://github.com/easybuilders/easybuild
# http://github.com/hpcugent/easybuild
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@ostueker This is wrong after the move to the easybuilders organisation, please change it back?

Comment thread easybuild/easyblocks/o/openbabel.py Outdated
@staticmethod
def extra_options():
extra_vars = {
'try_python': [True, "Try to build Open Babel's Python bindings. (-DPYTHON_BINDINGS=ON)", CUSTOM],
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What's the added benefit of introducing this?

If you don't want the Python bindings, it suffices to just exclude the use of Python as a dependency?

If there's a good use case for it, I'd rename it to with_python_bindings, since the try_ makes it sound like it's likely (& acceptable) to fail..

Copy link
Copy Markdown
Contributor Author

@ostueker ostueker Aug 23, 2017

Choose a reason for hiding this comment

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

it suffices to just exclude the use of Python as a dependency?

Unfortunately that does not work if Eigen has been built with a Python dependency as OpenBabel will inherit it indirectly.

Comment thread easybuild/easyblocks/o/openbabel.py Outdated
self.cfg['configopts'] += "-DPYTHON_INCLUDE_DIR=%s/include/python%s " % (root_python, shortpyver)
else:
self.log.info("Not enabling Python bindings")
self.with_python = False
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Already initialised as False, no point in setting it to False again?

Comment thread easybuild/easyblocks/o/openbabel.py Outdated
# since OpenBabel 2.4.0 the Python bindings under
# ${PREFIX}/lib/python2.7/site-packages rather than ${PREFIX}/lib
shortpyver = '.'.join(get_software_version('Python').split('.')[:2])
ob_pythonpath = 'lib/python%s/site-packages/' % (shortpyver)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

we have a dedicated function for this in the PythonPackage generic easyblock, so this can simply become:

ob_pythonpath = det_pylibdir()

In some cases it may be lib64 rather than lib, etc., which is taken into account by det_pylibdir()

@easybuilders easybuilders deleted a comment from boegelbot Dec 16, 2017
@boegel boegel added this to the 3.5.1 milestone Dec 16, 2017
@boegel boegel added the update label Dec 16, 2017
@boegel
Copy link
Copy Markdown
Member

boegel commented Dec 16, 2017

This looks good to go, tested it on all existing OpenBabel easyconfigs, works as expected.

I tried building OpenBabel 2.4.1 in order to have a test case for this, but the installation failed because most tests failed.

@ostueker Have you been able to come up with a working easyconfig for OpenBabel 2.4.1? And if so, can you share it?

@ostueker
Copy link
Copy Markdown
Contributor Author

@boegel: Our OpenBabel easyconfigs are located here
However our setup is quite customized (names of toolchains and certain prereqs are installed via NIX). But basically I resort to ignoring the tests as well. :-(

@boegel
Copy link
Copy Markdown
Member

boegel commented Dec 17, 2017

I figured out the problem I was running into with the tests in OpenBabel 2.4.1, was caused by commenting out the patch that reinstates $LD_LIBRARY_PATH for the tests, which is required.

You weren't hitting this issue @ostueker because I guess you're using RPATH?

PR for easyconfig in easybuilders/easybuild-easyconfigs#5533, this is good to go, thanks @ostueker!

@boegel boegel merged commit c990afe into easybuilders:develop Dec 17, 2017
@ostueker
Copy link
Copy Markdown
Contributor Author

@boegel : Yes, we're using RPATH.

@ostueker ostueker deleted the openbabel branch December 12, 2025 13:48
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