Acellera / moleculekit

MoleculeKit: Your favorite molecule manipulation kit
Other
198 stars 37 forks source link

SmallMolLib unexpected behaviour #112

Closed alejandrovr closed 2 years ago

alejandrovr commented 2 years ago

This returns a smallmol object with Hydrogens SmallMolLib(reflig_sdf, removeHs=True)[0]

This returns a smallmol object without hydrogens: SmallMolLib(reflig_sdf, removeHs=True, fixHs=False)[0]

fixHs is by default set to True, so hydrogens are "re-added" after removing then, which leads to unexpected behavior.

stefdoerr commented 2 years ago

True. But I'm a bit scared of changing this default :grimacing:

stefdoerr commented 2 years ago

Also because it might not be wrong. I think fixHs (aka AddHs) doesn't remove the existing ones (at least it doesn't call the function) so it could lead to different behaviour if you do removeHs=True & fixHs=True vs removeHs=False & fixHs=True. I'll need to test this a bit

alejandrovr commented 2 years ago

I see... take your time! ;)

stefdoerr commented 2 years ago

Finally we agreed to add logging instead of changing the defaults which makes it clear to the user what modifications have been done to the molecule https://github.com/Acellera/moleculekit/commit/0957519de75bd7d2043591b35c874f80e74375af