Merge pull request #220 from egonw/patch/aromatictyOverwriting

Patch/aromaticty overwriting
cdk · Aug 10, 2016 · 2f50ba9 · 2f50ba9
2 parents 1097093 + 1938e0b
commit 2f50ba9
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 6 deletions.
diff --git a/base/standard/src/main/java/org/openscience/cdk/tools/manipulator/AtomTypeManipulator.java b/base/standard/src/main/java/org/openscience/cdk/tools/manipulator/AtomTypeManipulator.java
@@ -28,8 +28,7 @@
 import org.openscience.cdk.interfaces.IPseudoAtom;
 
 /**
- * Class with utilities for the <code>AtomType</code> class.
- * - changed 21/7/05 by cho: add properties for mmff94 atom type
+ * Class with utilities for the {@link IAtomType} class.
  *
  * @author     mfe4
  * @author     egonw
@@ -40,7 +39,7 @@ public class AtomTypeManipulator {
 
     /**
      * Method that assign properties to an atom given a particular atomType.
-     * An <code>IllegalArgumentException</code> is thrown if the given <code>IAtomType</code>
+     * An {@link IllegalArgumentException} is thrown if the given {@link IAtomType}
      * is null. <b>This method overwrites non-null values.</b>
      *
      * @param  atom     Atom to configure
@@ -58,14 +57,14 @@ public static void configure(IAtom atom, IAtomType atomType) {
         // we set the atom type name, but nothing else
         atom.setAtomTypeName(atomType.getAtomTypeName());
 
-        // configuring aotm type information is not really valid
+        // configuring atom type information is not really valid
         // for pseudo atoms - first because they basically have no
         // type information and second because they may have information
         // associated with them from another context, which should not be
         // overwritten. So we only do the stuff below if we have a non pseudoatom
         //
         // a side effect of this is that it is probably not valid to get the atom
-        // type of a peudo atom. I think this is OK, since you can always check
+        // type of a pseudo atom. I think this is OK, since you can always check
         // whether an atom is a pseudo atom without looking at its atom type
         if (!(atom instanceof IPseudoAtom)) {
             atom.setSymbol(atomType.getSymbol());
@@ -82,7 +81,8 @@ public static void configure(IAtom atom, IAtomType atomType) {
             if (constant != null) {
                 atom.setProperty(CDKConstants.CHEMICAL_GROUP_CONSTANT, constant);
             }
-            atom.setFlag(CDKConstants.ISAROMATIC, atomType.getFlag(CDKConstants.ISAROMATIC));
+            if (atomType.getFlag(CDKConstants.ISAROMATIC))
+                atom.setFlag(CDKConstants.ISAROMATIC, atomType.getFlag(CDKConstants.ISAROMATIC));
 
             Object color = atomType.getProperty("org.openscience.cdk.renderer.color");
             if (color != null) {

diff --git a/...standard/src/test/java/org/openscience/cdk/tools/manipulator/AtomTypeManipulatorTest.java b/...standard/src/test/java/org/openscience/cdk/tools/manipulator/AtomTypeManipulatorTest.java
@@ -85,4 +85,32 @@ public void unknownAtomTypeDoesNotModifyProperties() {
         assertThat(atom.getSymbol(), is("C"));
         assertThat(atom.getAtomicNumber(), is(6));
     }
+
+    /**
+     * @cdk.bug 1322
+     */
+    @Test
+    public void aromaticityIsNotOverwritten() {
+        IAtom atom = new Atom(Elements.CARBON);
+        atom.setFlag(CDKConstants.ISAROMATIC, true);
+        IAtomType atomType = new AtomType(Elements.Unknown.toIElement());
+        atomType.setFlag(CDKConstants.ISAROMATIC, false);
+        atomType.setAtomTypeName("C.sp3");
+        AtomTypeManipulator.configure(atom, atomType);
+        assertThat(atom.getFlag(CDKConstants.ISAROMATIC), is(true));
+    }
+
+    /**
+     * @cdk.bug 1322
+     */
+    @Test
+    public void aromaticitySetIfForType() {
+        IAtom atom = new Atom(Elements.CARBON);
+        atom.setFlag(CDKConstants.ISAROMATIC, false);
+        IAtomType atomType = new AtomType(Elements.Unknown.toIElement());
+        atomType.setFlag(CDKConstants.ISAROMATIC, true);
+        atomType.setAtomTypeName("C.am");
+        AtomTypeManipulator.configure(atom, atomType);
+        assertThat(atom.getFlag(CDKConstants.ISAROMATIC), is(true));
+    }
 }