#535 Extended Nplanar3 perception

John May
cdk-1.4.x (181)
John May

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).



  • Egon Willighagen

    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...

  • Anonymous - 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.

    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);
        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
  • Egon Willighagen

    • status: open --> closed
  • Egon Willighagen

    OK, thanx all. Applied and pushed.

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


Log in to post a comment.

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:

JavaScript is required for this form.

No, thanks