josejimenezluna/delfta

Change order of 3D-check and hydrogen check; modify hydrogen check

Closed this issue · 4 comments

  • make3D automatically assigns hydrogens, so when passing a molecule without hydrogens but using force3D=True, there's no feedback in the logger that hydrogens have been added. Additionally, this means that force3d=True overrides addh=False. I suggest first checking if hydrogens are present, and, if that is not the case but force3d=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.

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.

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.

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.

Fixed with 1472e38