Learn how easy it is to sync an existing GitHub or Google Code repo to a SourceForge project! See Demo

Close

#535 Extended Nplanar3 perception

Needs_Review
closed
John May
cdk-1.4.x (181)
cdk-1.4.x
5
2012-10-28
2012-08-05
John May
No

Patch from Stephan Biesken - posted on cdk-devel:

...

The reason why the atom perception fails is because, in this particular case, it relies on aromaticity flags being already perceived for the atom bonds, which we get automatically from reading a SDF with bond order 4. The algorithm then checks the implicit hydrogen count and defaults (null) to N.sp2, which is incorrect. Obviously this issue can be solved with a few lines of code (patch attached, please review).

...

Discussion

  • These nitrogens are indeed very difficult... I need to see a unit test, with a small(-est possible) structure with the N.planar3 which fails and find N.sp2 instead...

    Also, it is not clear to my why the if clause checks if there is a charge value set, but not actually tests what the charge is...

     
  • SBeisken
    SBeisken
    2012-10-08

    Yes, Vedina, the other test is about the same issue. The big problem is really how to go from an aromatic structure to a proper single-double bond layout.

    For the record: N.sp2 has a piBondCount of 1, e.g., imine.
    N.planar3 has a piBondCount of 0, i.e. tertiary amine.

    For the representation of Pyrrole one would expect N.planar3 for the nitrogen.

    The following test fails with the unpatched code.

    Regarding the charge, I am as clueless as you. :) Can be removed.

    @Test
    public void test_n_planar3_sp2_aromaticity() throws CDKException {

        IChemObjectBuilder builder = SilentChemObjectBuilder.getInstance();
    
        // simulate an IAtomContainer returned from a SDFile with bond order 4 to indicate aromaticity
        IAtomContainer pyrrole = builder.newInstance(IAtomContainer.class);
    
        IAtom n1 = builder.newInstance(IAtom.class,"N");
        IAtom c2 = builder.newInstance(IAtom.class,"C");
        IAtom c3 = builder.newInstance(IAtom.class,"C");
        IAtom c4 = builder.newInstance(IAtom.class,"C");
        IAtom c5 = builder.newInstance(IAtom.class,"C");
    
        IBond b1 = builder.newInstance(IBond.class,n1, c2, IBond.Order.SINGLE);
        b1.setFlag(CDKConstants.ISAROMATIC, true);
        IBond b2 = builder.newInstance(IBond.class,c2, c3, IBond.Order.SINGLE);
        b2.setFlag(CDKConstants.ISAROMATIC, true);
        IBond b3 = builder.newInstance(IBond.class,c3, c4, IBond.Order.SINGLE);
        b3.setFlag(CDKConstants.ISAROMATIC, true);
        IBond b4 = builder.newInstance(IBond.class,c4, c5, IBond.Order.SINGLE);
        b4.setFlag(CDKConstants.ISAROMATIC, true);
        IBond b5 = builder.newInstance(IBond.class,c5, n1, IBond.Order.SINGLE);
        b5.setFlag(CDKConstants.ISAROMATIC, true);
    
        pyrrole.addAtom(n1);
        pyrrole.addAtom(c2);
        pyrrole.addAtom(c3);
        pyrrole.addAtom(c4);
        pyrrole.addAtom(c5);
        pyrrole.addBond(b1);
        pyrrole.addBond(b2);
        pyrrole.addBond(b3);
        pyrrole.addBond(b4);
        pyrrole.addBond(b5);
    
        AtomContainerManipulator.percieveAtomTypesAndConfigureAtoms(pyrrole);
        Assert.assertEquals(pyrrole.getAtom(0).getHybridization().name(), "PLANAR3");
    }
    
     
  • John May
    John May
    2012-10-10

    Okay I have reviewed (Stephan sent patch via email) and the unit test for pyrrole will fail on the existing code based but pass with the proposed modifications. I have added some documentation/extra test but will add as another tracker.

    I recommend this patch [attached] for master.

     
  • John May
    John May
    2012-10-26

    • branch: --> cdk-1.4.x
    • milestone: Needs_Revision --> Accepted
     
    • status: open --> closed
     
  • OK, thanx all. Applied and pushed.

    I did have to add a missing import and dependency, but otherwise is compiles fine.