Aromatic Oxygen (and Nitrogen, for that matter) #354
Conversation
An aromatic oxygen has two benzene bonds and no others bonds. Current implementation allows lone pairs to be incremented and decremented, presumably by a charge, though I'm not sure this would preserve aromaticity: To be aromatic the O must have at least one lone pair, in the p orbital, to contribute to the delocalized Pi system. This may be worth someone else checking.
…nd atom type. Previously only 6-membered rings of carbon were checked. Now, any sized ring, of any atom, is checked. If RDKit says it's an aromatic bond, then we make it an aromatic bond. If all the bonds in the ring are aromatic, then we add it to the list of isomers returned. We test that the oxygen in a furan ring is turned into an Ob AtomType. This tests both the getAromaticResonanceIsomers and the updateAtomTypes at once, which is not really the best unit test, but is better than nothing... More (and better) unit tests should be added.
Previously only 6-membered rings of Cb or Cbf were allowed. Now any sized ring, and any Xb or Xbf is allowed, as long as everything in the ring is Xb or Xbf. (i.e. Ob, Cb, Cbf, N3b, etc.) Perhaps we should also check Huckel rule of 4N+2 electrons in the ring? I suppose a manually crafted (via inputting an adjacency list) 5-membered ring of Cb atoms would pass this test, but be wrong. However, such a molecule would presumably not be made by RDKit, so should not show up via getAromaticResonanceIsomers(). So we're probably OK for now... We test that furan is detected as aromatic.
|
Currently failing two unit tests: ======================================================================
FAIL: Test the GroupAtom.equivalent() method.
----------------------------------------------------------------------
Traceback (most recent call last):
File "/Users/rwest/Code/RMG-Py/rmgpy/molecule/groupTest.py", line 146, in testEquivalent
self.assertTrue(atom1.equivalent(atom2), '{0!s} is not equivalent to {1!s}'.format(atom1, atom2))
AssertionError: ['Ob'] is not equivalent to ['O']
======================================================================
FAIL: Test the GroupAtom.isSpecificCaseOf() method.
----------------------------------------------------------------------
Traceback (most recent call last):
File "/Users/rwest/Code/RMG-Py/rmgpy/molecule/groupTest.py", line 174, in testIsSpecificCaseOf
self.assertTrue(atom1.isSpecificCaseOf(atom2), '{0!s} is not a specific case of {1!s}'.format(atom1, atom2))
AssertionError: ['Ob'] is not a specific case of ['O'] |
|
Will it be ok to allow rings more than length 6 to become aromatic? Do our group contributions account for that correctly? Also, is this a good time to reorganize atomTypes to be more intuitive and more transparent? We had talked about perhaps generating the list of atomTypes via a tree. We could do this as part of the database loading step to initialize the atomTypes. See previous issues #283 and #327 One thing I worry about is what the incrementBond, breakBond, loseLonePair, etc. attributes are used for. Are they being used to generate groups for detecting the reverse reaction? What kind of errors can writing these by hand lead to? |
|
Also the b in Cb stands for benzenic Ob as nomenclature might be confusing since it is furanic? |
|
I don't know what group contribution database will do. Maybe ring size should be capped at 6? I agree that #283 is still a good idea. Was @nickvandewiele working towards it in 75fb769? As for the naming convention... I don't think there is one that won't cause some confusion. Should the carbon atoms in a furan ring still be called |
|
Closing because this is extremely out of date. Aromatic atomtypes for oxygen and nitrogen have been implemented in #1206. However, aromatic resonance structure generation has not been extended to heteroatoms yet. There are other considerations which we will need to think about before taking that step. |
To try and address things like Furans, which should be aromatic, this topic branch (aromaticO) tries
to allow Ob atom types (and hopefully will enable the N3b and N5b atom types to function).
There is still some work to be done, but here is a start. Feel free to add commits to this branch and PR.