Skip to content
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

Bad handling of fragments in CIP code #4998

Closed
greglandrum opened this issue Feb 8, 2022 · 0 comments
Closed

Bad handling of fragments in CIP code #4998

greglandrum opened this issue Feb 8, 2022 · 0 comments
Labels
Milestone

Comments

@greglandrum
Copy link
Member

Describe the bug

The code tries to kekulize the molecule before doing its work and fails if the kekulization fails.
This is perfectly sensible for most molecules, but can cause problems if trying to do CIP assignment on fragments:

In [44]: m2  = Chem.MolFromSmarts('F[C@H](CNC)CCc(:c):c')

In [45]: m2.UpdatePropertyCache()

In [46]: Chem.AssignCIPLabels(m2)
[17:27:52] non-ring atom 7 marked aromatic
---------------------------------------------------------------------------
AtomKekulizeException                     Traceback (most recent call last)
<ipython-input-46-7ee1ccbf16f8> in <module>
----> 1 Chem.AssignCIPLabels(m2)

AtomKekulizeException: non-ring atom 7 marked aromatic

In this case, the CIP assignment is clearly defined and we should be able to generate it.

Things get more interesting with a molecule like this:

In [42]: m2  = Chem.MolFromSmarts('F[C@](Cl)c(:c):c')

In [43]: Chem.AssignCIPLabels(m2)
[16:36:35] non-ring atom 3 marked aromatic
---------------------------------------------------------------------------
AtomKekulizeException                     Traceback (most recent call last)
<ipython-input-43-7ee1ccbf16f8> in <module>
----> 1 Chem.AssignCIPLabels(m2)

AtomKekulizeException: non-ring atom 3 marked aromatic

Here we'll have to decide how to treat the aromatic bonds (since the CIP code only supports integral bond orders).

Configuration (please complete the following information):

  • RDKit version: master and current release
  • OS: all
@greglandrum greglandrum added the bug label Feb 8, 2022
greglandrum added a commit to greglandrum/rdkit that referenced this issue Feb 8, 2022
we should probably discuss this one
greglandrum added a commit to greglandrum/rdkit that referenced this issue Feb 10, 2022
we should probably discuss this one
@greglandrum greglandrum added this to the 2021_09_5 milestone Feb 12, 2022
greglandrum added a commit that referenced this issue Mar 5, 2022
* Fixes #4996

also switches to using the GraphMol version of catch_main.cpp so builds are faster

* Fixes #4998

we should probably discuss this one

* compare with previous results
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant