Skip to content

Bond parsing in PDB format now uses chain ids correctly #961

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
josemduarte merged 2 commits into from
Sep 20, 2021
Merged

Bond parsing in PDB format now uses chain ids correctly #961

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
15e1675
Fixing #943
josemduarte Aug 30, 2021
4e0473e
Docs
josemduarte Sep 3, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion biojava-integrationtest/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,20 @@
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
<scope>test</scope>
</dependency>
<!-- Junit5 dependencies, defined in parent pom. -->
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter-engine</artifactId>
</dependency>
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter-params</artifactId>
</dependency>
<dependency>
<groupId>org.junit.vintage</groupId>
<artifactId>junit-vintage-engine</artifactId>
</dependency>
<dependency>
<groupId>org.biojava</groupId>
<artifactId>biojava-structure</artifactId>
Expand Down
34 changes: 34 additions & 0 deletions ...ava-integrationtest/src/test/java/org/biojava/nbio/structure/test/io/TestBondParsing.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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());

}
}
60 changes: 43 additions & 17 deletions biojava-structure/src/main/java/org/biojava/nbio/structure/io/BondMaker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ public BondMaker(Structure structure, FileParsingParameters params) {
* nucleotide bonds: inferred from sequence and distances
* </li>
* <li>
* 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}
* </li>
*/
public void makeBonds() {
Expand Down Expand Up @@ -167,7 +167,7 @@ private void formIntraResidueBonds() {
continue;
}
// Now add support for altLocGroup
List<Group> totList = new ArrayList<Group>();
List<Group> totList = new ArrayList<>();
totList.add(mainGroup);
totList.addAll(mainGroup.getAltLocs());

Expand Down Expand Up @@ -293,10 +293,10 @@ public void formDisulfideBonds(List<SSBondImpl> disulfideBonds) {

private void formDisulfideBond(SSBondImpl disulfideBond) {
try {
Map<Integer, Atom> a = getAtomFromRecord("SG", "", "CYS",
Map<Integer, Atom> a = getAtomFromRecord("SG", "",
disulfideBond.getChainID1(), disulfideBond.getResnum1(),
disulfideBond.getInsCode1());
Map<Integer, Atom> b = getAtomFromRecord("SG", "", "CYS",
Map<Integer, Atom> b = getAtomFromRecord("SG", "",
disulfideBond.getChainID2(), disulfideBond.getResnum2(),
disulfideBond.getInsCode2());

Expand Down Expand Up @@ -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
Expand All @@ -333,15 +333,24 @@ public void formLinkRecordBond(LinkRecord linkRecord) {
return;

try {
Map<Integer, Atom> 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<Chain> nonpolyChains1 = structure.getNonPolyChainsByPDB(linkRecord.getChainID1());
List<Chain> nonpolyChains2 = structure.getNonPolyChainsByPDB(linkRecord.getChainID2());

Map<Integer, Atom> b = getAtomFromRecord(linkRecord.getName2(),
linkRecord.getAltLoc2(), linkRecord.getResName2(),
linkRecord.getChainID2(), linkRecord.getResSeq2(),
linkRecord.getiCode2());
List<String> allChainIds1 = new ArrayList<>();
List<String> 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<Integer, Atom> a = getAtomFromRecordTryMultipleChainIds(linkRecord.getName1(), linkRecord.getAltLoc1(), linkRecord.getResSeq1(), linkRecord.getiCode1(), allChainIds1);

Map<Integer, Atom> b = getAtomFromRecordTryMultipleChainIds(linkRecord.getName2(), linkRecord.getAltLoc2(), linkRecord.getResSeq2(), linkRecord.getiCode2(), allChainIds2);

for(int i=0; i<structure.nrModels(); i++){
if(a.containsKey(i) && b.containsKey(i)){
Expand All @@ -352,7 +361,7 @@ public void formLinkRecordBond(LinkRecord linkRecord) {
}
}
}
}catch (StructureException e) {
} catch (StructureException e) {
// Note, in Calpha only mode the link atoms may not be present.
if (! params.isParseCAOnly()) {
logger.warn("Could not find atoms specified in LINK record: {}",linkRecord.toString());
Expand All @@ -363,6 +372,23 @@ public void formLinkRecordBond(LinkRecord linkRecord) {
}
}

private Map<Integer, Atom> getAtomFromRecordTryMultipleChainIds(String name, String altLoc, String resSeq, String iCode, List<String> chainIds) throws StructureException {
Map<Integer, Atom> 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.
Expand Down Expand Up @@ -415,15 +441,15 @@ public void formBondsFromStructConn(StructConn conn) {
Map<Integer,Atom> a2 = null;

try {
a1 = getAtomFromRecord(atomName1, altLoc1, resName1, chainId1, seqId1, insCode1);
a1 = getAtomFromRecord(atomName1, altLoc1, chainId1, seqId1, insCode1);

} catch (StructureException e) {

logger.warn("Could not find atom specified in struct_conn record: {}{}({}) in chain {}, atom {} {}", seqId1, insCode1, resName1, chainId1, atomName1, altLocStr1);
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);
Expand Down Expand Up @@ -461,7 +487,7 @@ public void formBondsFromStructConn(StructConn conn) {
structure.setSSBonds(ssbonds);
}

private Map<Integer,Atom> getAtomFromRecord(String name, String altLoc, String resName, String chainID, String resSeq, String iCode)
private Map<Integer,Atom> getAtomFromRecord(String name, String altLoc, String chainID, String resSeq, String iCode)
throws StructureException {

if (iCode==null || iCode.isEmpty()) {
Expand Down