Updaetd chi index util to correctly evaluate deltav for sulphurs. Fix…

…es bug 3434741. Added unit test Signed-off-by: Egon Willighagen <egonw@users.sourceforge.net>
cdk · Nov 8, 2011 · 8225175 · 8225175
1 parent 434c9b1
commit 8225175
Show file tree

Hide file tree

Showing 2 changed files with 61 additions and 10 deletions.
diff --git a/src/main/org/openscience/cdk/qsar/descriptors/molecular/ChiIndexUtils.java b/src/main/org/openscience/cdk/qsar/descriptors/molecular/ChiIndexUtils.java
@@ -19,6 +19,8 @@
 package org.openscience.cdk.qsar.descriptors.molecular;
 
 import org.openscience.cdk.DefaultChemObjectBuilder;
+import org.openscience.cdk.annotations.TestClass;
+import org.openscience.cdk.annotations.TestMethod;
 import org.openscience.cdk.config.IsotopeFactory;
 import org.openscience.cdk.exception.CDKException;
 import org.openscience.cdk.interfaces.IAtom;
@@ -45,6 +47,7 @@
  * @cdk.module qsarmolecular
  * @cdk.githash
  */
+@TestClass("org.openscience.cdk.qsar.descriptors.molecular.ChiIndexUtilsTest")
 class ChiIndexUtils {
 
     /**
@@ -193,7 +196,8 @@ private static int getValenceElectronCount(IAtom atom) {
      * @return The empirical delta V if it is present in one of the above
      *         environments, -1 otherwise
      */
-    private static double deltavSulphur(IAtom atom, IAtomContainer atomContainer) {
+    @TestMethod("testDeltaVSuplhurSO,testDeltaVSulphurSO2")
+    protected static double deltavSulphur(IAtom atom, IAtomContainer atomContainer) {
         if (!atom.getSymbol().equals("S")) return -1;
 
         // check whether it's a S in S-S
@@ -204,21 +208,14 @@ private static double deltavSulphur(IAtom atom, IAtomContainer atomContainer) {
                 return .89;
         }
 
-        // check whether it's a S in -SO-
-        for (IAtom connectedAtom : connected) {
-            if (connectedAtom.getSymbol().equals("O")
-                    && atomContainer.getBond(atom, connectedAtom).getOrder() == IBond.Order.DOUBLE)
-                return 1.33;
-        }
-
-        // check whether it's a S in -SO2-
         int count = 0;
         for (IAtom connectedAtom : connected) {
             if (connectedAtom.getSymbol().equals("O")
                     && atomContainer.getBond(atom, connectedAtom).getOrder() == IBond.Order.DOUBLE)
                 count++;
         }
-        if (count == 2) return 2.67;
+        if (count == 1) return 1.33; // check whether it's a S in -SO-
+        else if (count == 2) return 2.67; // check whether it's a S in -SO2-
 
         return -1;
     }

diff --git a/src/test/org/openscience/cdk/qsar/descriptors/molecular/ChiIndexUtilsTest.java b/src/test/org/openscience/cdk/qsar/descriptors/molecular/ChiIndexUtilsTest.java
@@ -20,12 +20,66 @@
  */
 package org.openscience.cdk.qsar.descriptors.molecular;
 
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
 import org.openscience.cdk.CDKTestCase;
+import org.openscience.cdk.DefaultChemObjectBuilder;
+import org.openscience.cdk.interfaces.IAtom;
+import org.openscience.cdk.interfaces.IBond;
+import org.openscience.cdk.interfaces.IMolecule;
 
 /**
  * @cdk.module test-qsarmolecular
  */
 public class ChiIndexUtilsTest extends CDKTestCase {
 
+    DefaultChemObjectBuilder builder;
+
+    public ChiIndexUtilsTest() {
+    }
+
+    @Before
+    public void setup() {
+        builder = (DefaultChemObjectBuilder) DefaultChemObjectBuilder.getInstance();
+    }
+
+    @Test
+    public void testDeltaVSulphurSO() {
+        IAtom s = builder.newInstance(IAtom.class, "S");
+        IAtom o = builder.newInstance(IAtom.class, "O");
+        IBond b = builder.newInstance(IBond.class, s, o);
+        b.setOrder(IBond.Order.DOUBLE);
+
+        IMolecule m = builder.newInstance(IMolecule.class);
+        m.addAtom(s);
+        m.addAtom(o);
+        m.addBond(b);
+
+        double deltav = ChiIndexUtils.deltavSulphur(s, m);
+        Assert.assertEquals(1.33, deltav, 0.01);
+    }
+
+    @Test
+    public void testDeltaVSulphurSO2() {
+        IAtom s = builder.newInstance(IAtom.class, "S");
+        IAtom o1 = builder.newInstance(IAtom.class, "O");
+        IAtom o2 = builder.newInstance(IAtom.class, "O");
+        IBond b1 = builder.newInstance(IBond.class, s, o1);
+        IBond b2 = builder.newInstance(IBond.class, s, o2);
+        b1.setOrder(IBond.Order.DOUBLE);
+        b2.setOrder(IBond.Order.DOUBLE);
+
+        IMolecule m = builder.newInstance(IMolecule.class);
+        m.addAtom(s);
+        m.addAtom(o1);
+        m.addBond(b1);
+        m.addAtom(o2);
+        m.addBond(b2);
+
+        double deltav = ChiIndexUtils.deltavSulphur(s, m);
+        Assert.assertEquals(2.67, deltav, 0.01);
+    }
+
 }