Change order of 3D-check and hydrogen check; modify hydrogen check
Closed this issue · 4 comments
cisert commented
make3D
automatically assigns hydrogens, so when passing a molecule without hydrogens but usingforce3D=True
, there's no feedback in the logger that hydrogens have been added. Additionally, this means thatforce3d=True
overridesaddh=False
. I suggest first checking if hydrogens are present, and, if that is not the case butforce3d=True
, to return an error.- Checking for hydrogens with
1 in set([atom.atomicnum for atom in mol.atoms])
runs the risk of ignoring molecules where only certain hydrogens are present. It's a strange edge case, but I suggest checking if e.g.[atom.atomicnum for atom in mol.atoms]
changes after adding hydrogens to a copy of the molecule. Not elegant, but didn't find an alternative yet.
josejimenezluna commented
Good first point.
About the second, should we respect if the user has already assigned hydrogens to a mol, even if they don't correspond with OBabel's choice? Sounds a bit too aggressive to me.
cisert commented
Hm, depends. I think the case where only some of the hydrogens are explicit but others are not would likely give strange results, since all the molecules in the training set had all their hydrogens added. Although that would lie in the user's responsibility, if they decide to explicitly set addh=False
.
cisert commented
I'd make addh=True
by default though - it doesn't change anything if hydrogens are present, and if they are not we add them. If someone disabled that and get's strange results, that's on them.
josejimenezluna commented
Fixed with 1472e38