mosdef-hub / mbuild

A hierarchical, component based molecule builder
https://mbuild.mosdef.org
Other
173 stars 80 forks source link

Warnings when loading compound from SMILES string #706

Closed rsdefever closed 4 years ago

rsdefever commented 4 years ago

There are a couple of warnings that get thrown when loading an mbuild.Compound from a SMILES string. E.g.,

import mbuild
molecule = mbuild.load("C",smiles=True)

The two warnings are:

/Users/ryandefever/anaconda3/envs/mosdef-dev/lib/python3.7/site-packages/openbabel/__init__.py:14: UserWarning: "import openbabel" is deprecated, instead use "from openbabel import openbabel"
  warnings.warn('"import openbabel" is deprecated, instead use "from openbabel import openbabel"')

And:

/Users/ryandefever/repos/mosdef/mbuild/mbuild/compound.py:2749: UserWarning: No unitcell detected for pybel.Molecule C

The first appears to come from openbabel, but I don't quite understand why. I tried changing our import statement in utils/io.py from import openbabel to from openbabel import openbabel, but the warning remains.

The second looks like something in our from_pybel function in the Compound class. Do we really need to be throwing a warning here? At least in the use case of creating compounds from SMILES strings this seems extraneous. I think this fits into further discussions we need to have about the role of a box in an mbuild.Compound.

mattwthompson commented 4 years ago
  1. I think our soft import of babel is the cause here - it may take some updates to the import_ function to actually do from openbabel import openbabel but it would be worthwhile

  2. I am not sure this warning should be raised - it's all good and well that babel's conception of a "molecule" includes a box but this isn't so central to the design of a mbuild.Compound that it absolutely must be a part of the conversion. And it seems like generating a babel molecule from a SMILES string doesn't usually include a box? I wonder if this warning is triggered 100% of the time. Other conversions from babel, if we want to use them, maybe should raise this warning.