diff --git a/biojava-integrationtest/pom.xml b/biojava-integrationtest/pom.xml index 0bd381f989..bb1a97c71b 100644 --- a/biojava-integrationtest/pom.xml +++ b/biojava-integrationtest/pom.xml @@ -23,8 +23,20 @@ junit junit - test + + + org.junit.jupiter + junit-jupiter-engine + + + org.junit.jupiter + junit-jupiter-params + + + org.junit.vintage + junit-vintage-engine + org.biojava biojava-structure diff --git a/biojava-integrationtest/src/test/java/org/biojava/nbio/structure/test/io/TestBondParsing.java b/biojava-integrationtest/src/test/java/org/biojava/nbio/structure/test/io/TestBondParsing.java new file mode 100644 index 0000000000..7be8b523a5 --- /dev/null +++ b/biojava-integrationtest/src/test/java/org/biojava/nbio/structure/test/io/TestBondParsing.java @@ -0,0 +1,34 @@ +package org.biojava.nbio.structure.test.io; + +import org.biojava.nbio.structure.Atom; +import org.biojava.nbio.structure.Group; +import org.biojava.nbio.structure.Structure; +import org.biojava.nbio.structure.io.FileParsingParameters; +import org.biojava.nbio.structure.io.PDBFileReader; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.*; + +class TestBondParsing { + + /** + * Integration test for bond parsing in PDB-format, where author chain ids and asym ids differ and can cause + * problems. See https://github.com/biojava/biojava/issues/943 + */ + @Test + public void testIssue943() throws Exception { + PDBFileReader reader = new PDBFileReader(); + FileParsingParameters params = new FileParsingParameters(); + params.setCreateAtomBonds(true); + reader.setFileParsingParameters(params); + Structure s = reader.getStructureById("1v9i"); + + Group his95 = s.getPolyChain("A").getAtomGroup(94); + Atom ne2His95 = his95.getAtom("NE2"); + assertEquals(3, ne2His95.getBonds().size()); + + Group zn = s.getNonPolyChain("B").getAtomGroup(0); + assertEquals(3, zn.getAtom("ZN").getBonds().size()); + + } +} 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 index 7c4fe74144..27e586667d 100644 --- 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 @@ -96,7 +96,7 @@ public BondMaker(Structure structure, FileParsingParameters params) { * nucleotide bonds: inferred from sequence and distances * *
  • - * intra-group (residue) bonds: read from the chemical component dictionary, via {@link ChemCompProvider} + * intra-group (residue) bonds: read from the chemical component dictionary, via {@link org.biojava.nbio.structure.chem.ChemCompProvider} *
  • */ public void makeBonds() { @@ -167,7 +167,7 @@ private void formIntraResidueBonds() { continue; } // Now add support for altLocGroup - List totList = new ArrayList(); + List totList = new ArrayList<>(); totList.add(mainGroup); totList.addAll(mainGroup.getAltLocs()); @@ -293,10 +293,10 @@ public void formDisulfideBonds(List disulfideBonds) { private void formDisulfideBond(SSBondImpl disulfideBond) { try { - Map a = getAtomFromRecord("SG", "", "CYS", + Map a = getAtomFromRecord("SG", "", disulfideBond.getChainID1(), disulfideBond.getResnum1(), disulfideBond.getInsCode1()); - Map b = getAtomFromRecord("SG", "", "CYS", + Map b = getAtomFromRecord("SG", "", disulfideBond.getChainID2(), disulfideBond.getResnum2(), disulfideBond.getInsCode2()); @@ -324,7 +324,7 @@ private void formDisulfideBond(SSBondImpl disulfideBond) { /** * Creates bond objects from a LinkRecord as parsed from a PDB file - * @param linkRecord + * @param linkRecord the PDB-format LINK record */ public void formLinkRecordBond(LinkRecord linkRecord) { // only work with atoms that aren't alternate locations @@ -333,15 +333,24 @@ public void formLinkRecordBond(LinkRecord linkRecord) { return; try { - Map a = getAtomFromRecord(linkRecord.getName1(), - linkRecord.getAltLoc1(), linkRecord.getResName1(), - linkRecord.getChainID1(), linkRecord.getResSeq1(), - linkRecord.getiCode1()); + // The PDB format uses author chain ids to reference chains. But one author chain id corresponds to multiple asym ids, + // thus we need to grab all the possible asym ids (poly and nonpoly) and then try to find the atoms + // See issue https://github.com/biojava/biojava/issues/943 + String polyChainId1 = structure.getPolyChainByPDB(linkRecord.getChainID1()).getId(); + String polyChainId2 = structure.getPolyChainByPDB(linkRecord.getChainID2()).getId(); + List nonpolyChains1 = structure.getNonPolyChainsByPDB(linkRecord.getChainID1()); + List nonpolyChains2 = structure.getNonPolyChainsByPDB(linkRecord.getChainID2()); - Map b = getAtomFromRecord(linkRecord.getName2(), - linkRecord.getAltLoc2(), linkRecord.getResName2(), - linkRecord.getChainID2(), linkRecord.getResSeq2(), - linkRecord.getiCode2()); + List allChainIds1 = new ArrayList<>(); + List allChainIds2 = new ArrayList<>(); + if (polyChainId1!=null) allChainIds1.add(polyChainId1); + if (polyChainId2!=null) allChainIds2.add(polyChainId2); + if (nonpolyChains1!=null) nonpolyChains1.forEach(npc -> allChainIds1.add(npc.getId())); + if (nonpolyChains2!=null) nonpolyChains2.forEach(npc -> allChainIds2.add(npc.getId())); + + Map a = getAtomFromRecordTryMultipleChainIds(linkRecord.getName1(), linkRecord.getAltLoc1(), linkRecord.getResSeq1(), linkRecord.getiCode1(), allChainIds1); + + Map b = getAtomFromRecordTryMultipleChainIds(linkRecord.getName2(), linkRecord.getAltLoc2(), linkRecord.getResSeq2(), linkRecord.getiCode2(), allChainIds2); for(int i=0; i getAtomFromRecordTryMultipleChainIds(String name, String altLoc, String resSeq, String iCode, List chainIds) throws StructureException { + Map a = null; + for (String chainId : chainIds) { + try { + a = getAtomFromRecord(name, altLoc, chainId, resSeq, iCode); + // first instance that doesn't give an exception will be considered the right one. Not much more we can do here + break; + } catch (StructureException e) { + logger.debug("Tried to get atom {} {} {} (alt loc {}) from chain id {}, but did not find it", name, resSeq, iCode, altLoc, chainId); + } + } + if (a == null) { + throw new StructureException("Could not find atom "+name+" "+resSeq+" "+iCode+" (alt loc "+altLoc+")"); + } + return a; + } + public void formBondsFromStructConn(StructConn conn) { final String symop = "1_555"; // For now - accept bonds within origin asymmetric unit. @@ -415,7 +441,7 @@ public void formBondsFromStructConn(StructConn conn) { Map a2 = null; try { - a1 = getAtomFromRecord(atomName1, altLoc1, resName1, chainId1, seqId1, insCode1); + a1 = getAtomFromRecord(atomName1, altLoc1, chainId1, seqId1, insCode1); } catch (StructureException e) { @@ -423,7 +449,7 @@ public void formBondsFromStructConn(StructConn conn) { continue; } try { - a2 = getAtomFromRecord(atomName2, altLoc2, resName2, chainId2, seqId2, insCode2); + a2 = getAtomFromRecord(atomName2, altLoc2, chainId2, seqId2, insCode2); } catch (StructureException e) { logger.warn("Could not find atom specified in struct_conn record: {}{}({}) in chain {}, atom {} {}", seqId2, insCode2, resName2, chainId2, atomName2, altLocStr2); @@ -461,7 +487,7 @@ public void formBondsFromStructConn(StructConn conn) { structure.setSSBonds(ssbonds); } - private Map getAtomFromRecord(String name, String altLoc, String resName, String chainID, String resSeq, String iCode) + private Map getAtomFromRecord(String name, String altLoc, String chainID, String resSeq, String iCode) throws StructureException { if (iCode==null || iCode.isEmpty()) {