-
Notifications
You must be signed in to change notification settings - Fork 47
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Problem loading aromatic SMILES or kekulized molecules with RDKit #396
Comments
|
Hey Lukas, just wanted to bring this issue up again as it has appeared quite a few times in our work. I was planning on submitting a PR in which a warning is issued to the user if an aromatic molecule is received in the |
Issues: #396 Added additional notes section in the docstring to explain that users should kekulize their molecule prior to calling the `init_form_rdkit_mol` method. It also explains the consequences of not doing so (bond orders changed to integer values). Co-authored-by: stevenbennett96 <[email protected]>
There is an issue with loading aromatic SMILES or kekulized molecules from RDKit where bonds involved in aromatic rings are converted to single bonds.
Discovered by @joshkamm, @andrewtarzia and myself.
stk
is able to handle to handle the aromatic bond order of 1.5, however, when converting back into the RDKit molecule, or writing to a.mol
file, the aromatic bond information is lost.When converting back into the RDKit molecule with
to_rdkit_mol
, I believe this occurs due to line 739 in https://github.com/lukasturcani/stk/blob/5987b0aa7ce73175701ba267d392b4fe5c651707/src/stk/molecular/molecules/molecule/molecule.py. Whenrdkit.BondType
is called with a floating point bond order, it is converted into an int.This behaviour can be seen just by executing
For writing to a
.mol
file, this issue appears due to line 82 in https://github.com/andrewtarzia/stk/blob/main/src/stk/molecular/molecules/molecule/utilities/writers/mdl_mol.py, as mentioned by @andrewtarzia on Discord as bond orders are all converted to ints.To reproduce this, the following code can be used:
To fix, maybe an error can be raised if the user passes in non-integer bond types when initialising from an RDKit molecule, or some different handling of non-integer bond types within
stk
itself.The text was updated successfully, but these errors were encountered: