Refactored ChargeAdder and fixed bugs where it wasn't adding charges to all groups.

abradle · josemduarte · commit 22ab7d4ccc73 · 2016-04-29T10:29:09.000-07:00
Changed the method to a static. And updated the PDBFileParser and the SimpleMMCifConsumer.
Added a basic test to check that charges are being added appropriately.
diff --git a/biojava-structure/src/main/java/org/biojava/nbio/structure/io/ChargeAdder.java b/biojava-structure/src/main/java/org/biojava/nbio/structure/io/ChargeAdder.java
@@ -34,38 +34,49 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+/**
+ * A class to add appropriate charge information to a structure.
+ * @author Anthony Bradley
+ *
+ */
 public class ChargeAdder {
 
 	private static final Logger logger = LoggerFactory.getLogger(ChargeAdder.class);
 
-	private Structure structure;
-
-	public ChargeAdder(Structure structure) {
-		this.structure = structure;
-	}
+	/**
+	 * Function to add the charges to a given structure.
+	 */
+	public static void addCharges(Structure structure) {
+		// Loop through the models
+		for(int i=0; i<structure.nrModels(); i++){
+			for(Chain c: structure.getChains(i)){
+				for(Group g: c.getAtomGroups()){
+					ChemComp thisChemComp = ChemCompGroupFactory.getChemComp(g.getPDBName());
+					List<ChemCompAtom> chemAtoms = thisChemComp.getAtoms();
+					for(ChemCompAtom chemCompAtom : chemAtoms) {
+						Atom atom = g.getAtom(chemCompAtom.getAtom_id());
 
-	public void addCharges() {
-		// Only adds charges to the first MODEL
-		for(Chain c: structure.getChains()){
-			for(Group g: c.getAtomGroups()){
-				ChemComp thisChemComp = ChemCompGroupFactory.getChemComp(g.getPDBName());
-				List<ChemCompAtom> chemAtoms = thisChemComp.getAtoms();
-				List<Atom> protAtoms = g.getAtoms();
-				if(protAtoms.size()!=chemAtoms.size()){
-					continue;
-				}
-				for(int i=0; i<protAtoms.size();i++){
-					ChemCompAtom cca = chemAtoms.get(i);
-					Atom a = protAtoms.get(i);
-					// Get the charge
-					try {
-						short charge = Short.parseShort(cca.getCharge());
-						a.setCharge(charge);
-					} catch (NumberFormatException e) {
-						logger.info("Could not parse charge for atom {} of {}. Will set its charge to 0", a.getName(), g.getPDBName());
-						a.setCharge((short)0);
+						String stringCharge = chemCompAtom.getCharge();
+						short shortCharge = 0;
+						if (stringCharge!=null){
+							shortCharge = Short.parseShort(stringCharge);
+						}
+						else{
+							logger.warn("Null charge on atom "+chemCompAtom.getAtom_id()+" in group "+thisChemComp.getId());
+						}
+						if(atom!=null){
+							atom.setCharge(shortCharge);
+						}
+						// Now do the same for alt locs
+						for (Group altLoc : g.getAltLocs()) {
+							Atom altAtom = altLoc.getAtom(chemCompAtom.getAtom_id());
+							if(altAtom!=null){
+								altAtom.setCharge(shortCharge);
+							}
+						}
 					}
 				}
+
 			}
 		}
 	}
diff --git a/biojava-structure/src/main/java/org/biojava/nbio/structure/io/PDBFileParser.java b/biojava-structure/src/main/java/org/biojava/nbio/structure/io/PDBFileParser.java
@@ -2764,8 +2764,7 @@ else if ( params.isParseSecStruc()) {
 			}
 
 	private void addCharges() {
-		ChargeAdder adder = new ChargeAdder(structure);
-		adder.addCharges();
+		ChargeAdder.addCharges(structure);
 	}
 
 	/**
diff --git a/biojava-structure/src/main/java/org/biojava/nbio/structure/io/mmcif/SimpleMMcifConsumer.java b/biojava-structure/src/main/java/org/biojava/nbio/structure/io/mmcif/SimpleMMcifConsumer.java
@@ -992,8 +992,7 @@ private void linkCompounds() {
 	}
 
 	private void addCharges() {
-		ChargeAdder adder = new ChargeAdder(structure);
-		adder.addCharges();
+		ChargeAdder.addCharges(structure);
 	}
 
 	/**
diff --git a/biojava-structure/src/test/java/org/biojava/nbio/structure/TestChargeAdder.java b/biojava-structure/src/test/java/org/biojava/nbio/structure/TestChargeAdder.java
@@ -0,0 +1,43 @@
+package org.biojava.nbio.structure;
+
+import static org.junit.Assert.assertEquals;
+
+import java.io.IOException;
+
+import org.biojava.nbio.structure.io.ChargeAdder;
+import org.junit.Test;
+
+/**
+ * Class of functions to test the charge adder.
+ * @author Anthony Bradley
+ *
+ */
+public class TestChargeAdder {
+
+	
+	/**
+	 * Test that it works on a very basic level.
+	 * @throws StructureException 
+	 * @throws IOException 
+	 */
+	@Test
+	public void testBasic() throws IOException, StructureException {
+		
+		// Get the structure
+		Structure structure = StructureIO.getStructure("3AAE");
+		ChargeAdder.addCharges(structure);
+		// Now count the charges
+		int chargeCount = 0;
+		for(Chain chain : structure.getChains()) {
+			for (Group group : chain.getAtomGroups()) {
+				for (Atom atom : group.getAtoms()) {
+					if (atom.getCharge()!=0) {
+						chargeCount++;
+					}
+				}
+			}
+		}
+		// Check that the count is as excpected
+		assertEquals(chargeCount, 425);	
+	}
+}

Original file line number	Diff line number	Diff line change
`@@ -2764,8 +2764,7 @@ else if ( params.isParseSecStruc()) {`
`2764`	`2764`	`}`
`2765`	`2765`
`2766`	`2766`	`private void addCharges() {`
`2767`		`- ChargeAdder adder = new ChargeAdder(structure);`
`2768`		`- adder.addCharges();`
	`2767`	`+ ChargeAdder.addCharges(structure);`
`2769`	`2768`	`}`
`2770`	`2769`
`2771`	`2770`	`/**`
Original file line number	Diff line number	Diff line change
`@@ -992,8 +992,7 @@ private void linkCompounds() {`
`992`	`992`	`}`
`993`	`993`
`994`	`994`	`private void addCharges() {`
`995`		`- ChargeAdder adder = new ChargeAdder(structure);`
`996`		`- adder.addCharges();`
	`995`	`+ ChargeAdder.addCharges(structure);`
`997`	`996`	`}`
`998`	`997`
`999`	`998`	`/**`