Fixed a bug in generation of disulphide bonds - including a test case.

abradle · abradle · commit 636929bce3d2 · 2016-08-15T09:46:10.000-07:00
diff --git a/biojava-structure/src/main/java/org/biojava/nbio/structure/io/BondMaker.java b/biojava-structure/src/main/java/org/biojava/nbio/structure/io/BondMaker.java
@@ -281,8 +281,10 @@ private void formDisulfideBond(SSBondImpl disulfideBond) {
 				if(a.containsKey(i) && b.containsKey(i)){
 					// TODO determine what the actual bond order of this bond is; for
 					// now, we're assuming they're single bonds
-					Bond ssbond =  new BondImpl(a.get(i), b.get(i), 1);
-					structure.addSSBond(ssbond);
+					if(!a.get(i).equals(b.get(i))){
+						Bond ssbond =  new BondImpl(a.get(i), b.get(i), 1);
+						structure.addSSBond(ssbond);
+					}
 				}
 			}
 
@@ -322,7 +324,9 @@ public void formLinkRecordBond(LinkRecord linkRecord) {
 				if(a.containsKey(i) && b.containsKey(i)){
 					// TODO determine what the actual bond order of this bond is; for
 					// now, we're assuming they're single bonds
-					new BondImpl(a.get(i), b.get(i), 1);
+					if(!a.get(i).equals(b.get(i))){
+						new BondImpl(a.get(i), b.get(i), 1);
+					}
 				}
 			}
 		}catch (StructureException e) {
@@ -336,7 +340,7 @@ public void formLinkRecordBond(LinkRecord linkRecord) {
 		}
 	}
 
-	
+
 	public void formBondsFromStructConn(List<StructConn> structConn) {
 
 		final String symop = "1_555"; // For now - accept bonds within origin asymmetric unit.
@@ -413,7 +417,9 @@ public void formBondsFromStructConn(List<StructConn> structConn) {
 			for(int i=0; i<structure.nrModels(); i++){
 				Bond bond = null;
 				if(a1.containsKey(i) && a2.containsKey(i)){
-					bond = new BondImpl(a1.get(i), a2.get(i), 1);
+					if(!a1.get(i).equals(a2.get(i))){
+						bond = new BondImpl(a1.get(i), a2.get(i), 1);
+					}
 				}
 				if(bond!=null){
 					if (conn.getConn_type_id().equals("disulf")) {
diff --git a/biojava-structure/src/test/java/org/biojava/nbio/structure/TestBond.java b/biojava-structure/src/test/java/org/biojava/nbio/structure/TestBond.java
@@ -228,7 +228,37 @@ private int testMissingBonds(String pdbId) throws IOException, StructureExceptio
 			}
 		}
 		return nonBondedCounter;
+	}
 
+	private int testBondedToSelf(String pdbId) throws IOException, StructureException {
+		Structure inputStructure = StructureIO.getStructure(pdbId);
+		int bondedToSelf =0;
+		for(int i=0;i<inputStructure.nrModels();i++){
+			for(Chain c: inputStructure.getChains(i)){
+				for(Group g: c.getAtomGroups()){
+					// Skip single atom groups
+					if(g.size()<=1){
+						continue;
+					}
+					// Get all the atoms
+					List<Atom> atomsList = new ArrayList<>(g.getAtoms());
+					for(Group altLocOne: g.getAltLocs()){
+						atomsList.addAll(altLocOne.getAtoms());
+					}
+					// Check they all have bonds
+					for(Atom a: atomsList){
+						if(a.getBonds()!=null){
+						for(Bond b: a.getBonds()){
+							if(b.getAtomA().equals(b.getAtomB())){
+								bondedToSelf+=1;
+							}
+						}
+						}
+					}
+				}
+			}
+		}
+		return bondedToSelf;
 	}
 
 
@@ -286,4 +316,12 @@ public void testWeirdCase() throws IOException, StructureException {
 		assertEquals(testMissingBonds("1IU6"),6);
 	}
 
+
+	@Test
+	public void testSSBonds() throws IOException, StructureException {
+		for(String pdbCode : new String[]{"3ZXW",	"1NTY", "4H2I", "2K6D", "2MLM"}){
+			assertEquals(testBondedToSelf(pdbCode),0);
+		}
+	}
+
 }

Original file line number	Diff line number	Diff line change
`@@ -281,8 +281,10 @@ private void formDisulfideBond(SSBondImpl disulfideBond) {`
`281`	`281`	`if(a.containsKey(i) && b.containsKey(i)){`
`282`	`282`	`// TODO determine what the actual bond order of this bond is; for`
`283`	`283`	`// now, we're assuming they're single bonds`
`284`		`- Bond ssbond = new BondImpl(a.get(i), b.get(i), 1);`
`285`		`- structure.addSSBond(ssbond);`
	`284`	`+ if(!a.get(i).equals(b.get(i))){`
	`285`	`+ Bond ssbond = new BondImpl(a.get(i), b.get(i), 1);`
	`286`	`+ structure.addSSBond(ssbond);`
	`287`	`+ }`
`286`	`288`	`}`
`287`	`289`	`}`
`288`	`290`
`@@ -322,7 +324,9 @@ public void formLinkRecordBond(LinkRecord linkRecord) {`
`322`	`324`	`if(a.containsKey(i) && b.containsKey(i)){`
`323`	`325`	`// TODO determine what the actual bond order of this bond is; for`
`324`	`326`	`// now, we're assuming they're single bonds`
`325`		`- new BondImpl(a.get(i), b.get(i), 1);`
	`327`	`+ if(!a.get(i).equals(b.get(i))){`
	`328`	`+ new BondImpl(a.get(i), b.get(i), 1);`
	`329`	`+ }`
`326`	`330`	`}`
`327`	`331`	`}`
`328`	`332`	`}catch (StructureException e) {`
`@@ -336,7 +340,7 @@ public void formLinkRecordBond(LinkRecord linkRecord) {`
`336`	`340`	`}`
`337`	`341`	`}`
`338`	`342`
`339`		`-`
	`343`	`+`
`340`	`344`	`public void formBondsFromStructConn(List<StructConn> structConn) {`
`341`	`345`
`342`	`346`	`final String symop = "1_555"; // For now - accept bonds within origin asymmetric unit.`
`@@ -413,7 +417,9 @@ public void formBondsFromStructConn(List<StructConn> structConn) {`
`413`	`417`	`for(int i=0; i<structure.nrModels(); i++){`
`414`	`418`	`Bond bond = null;`
`415`	`419`	`if(a1.containsKey(i) && a2.containsKey(i)){`
`416`		`- bond = new BondImpl(a1.get(i), a2.get(i), 1);`
	`420`	`+ if(!a1.get(i).equals(a2.get(i))){`
	`421`	`+ bond = new BondImpl(a1.get(i), a2.get(i), 1);`
	`422`	`+ }`
`417`	`423`	`}`
`418`	`424`	`if(bond!=null){`
`419`	`425`	`if (conn.getConn_type_id().equals("disulf")) {`