Description
This issue is inspired by a hotfix I quickly wrote to solve a strange openbabel import error, but I thought it was worth a wider discussion because of a few more issues that are worth addressing.
Currently, in a few places (at least cclib/io/filewriter.py and cclib/io/ccio.py) we do something like this:
_has_openbabel = find_package("openbabel")
if _has_openbabel:
from cclib.bridge import makeopenbabel
# Open Babel 3.0 and above
try:
from openbabel import openbabel as ob
import openbabel.pybel as pb
# Open Babel 2.4.x and below
except:
import openbabel as ob
import pybel as pbThe specific error that I've fixed is that it's possible (at least in older Babel 2.0) to install openbabel but not pybel. In this case, filewriter.py is not importable. More importantly, through a series of import hops, filewriter gets imported from the main cclib __init__.py file, meaning that no part of cclib is usable on such a system (even though the openbabel bits probably aren't getting used).
The current patch is something like:
_has_openbabel = find_package("openbabel")
if _has_openbabel:
from cclib.bridge import makeopenbabel
# Open Babel 3.0 and above
try:
from openbabel import openbabel as ob
import openbabel.pybel as pb
# Open Babel 2.4.x and below
except:
import openbabel as ob
try:
import pybel as pb
except ModuleNotFoundError:
_has_openbabel = FalseBut this is really a bit backwards, more properly we should be doing something like:
try:
try:
# Babel 3.x
from openbabel import openbabel as ob
import openbabel.pybel as pb
_has_openbabel = True
except ModuleNotFoundError:
# Babel 2.x
import openbabel as ob
import pybel as pb
_has_openbabel = True
except Exception:
# Maybe log an error.
_has_openbabel = FalseMore generally though, it feels strange to be importing openbabel at the module level at all, considering it's not formally a dependency of cclib and it's mostly used for fallback stuff afaik? Having import cclib end up doing import openbabel was a bit of a surprise for me at least...