Skip to content

Commit 12f4e07

Browse files
andreasprlicandreasprlic
authored
Merge pull request #541 from josemduarte/fixIssue517
Fix for issue 517
2 parents 9a28b2b + 347a8fc commit 12f4e07

8 files changed

Lines changed: 18745 additions & 20881 deletions

File tree

‎biojava-integrationtest/src/test/resources/1tap.pdb‎

Lines changed: 18678 additions & 20837 deletions
Large diffs are not rendered by default.

‎biojava-structure/src/main/java/org/biojava/nbio/structure/AminoAcidImpl.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,10 @@ public Object clone() {
183183
n.addAltLoc(nAltLocGroup);
184184
}
185185
}
186+
187+
if (chemComp!=null)
188+
n.setChemComp(chemComp);
189+
186190

187191
return n;
188192
}

‎biojava-structure/src/main/java/org/biojava/nbio/structure/HetatomImpl.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ public static enum PerformanceBehavior {
9090

9191
private Map<String,Atom> atomNameLookup;
9292

93-
private ChemComp chemComp ;
93+
protected ChemComp chemComp ;
9494

9595
private List<Group> altLocs;
9696

@@ -403,6 +403,9 @@ public Object clone() {
403403
n.addAltLoc(nAltLocGroup);
404404
}
405405
}
406+
407+
if (chemComp!=null)
408+
n.setChemComp(chemComp);
406409

407410
return n;
408411
}

‎biojava-structure/src/main/java/org/biojava/nbio/structure/NucleotideImpl.java‎

Lines changed: 1 addition & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -93,29 +93,5 @@ public Atom getP() {
9393

9494
}
9595

96-
@Override
97-
public Object clone(){
98-
NucleotideImpl n = new NucleotideImpl();
99-
100-
n.setPDBFlag(has3D());
101-
n.setResidueNumber(getResidueNumber());
102-
103-
n.setPDBName(getPDBName());
104-
105-
// copy the atoms
106-
for (Atom atom1 : atoms) {
107-
Atom atom = (Atom) atom1.clone();
108-
n.addAtom(atom);
109-
atom.setGroup(n);
110-
}
111-
112-
// copying the alt loc groups if present, otherwise they stay null
113-
if (getAltLocs()!=null && !getAltLocs().isEmpty()) {
114-
for (Group altLocGroup:this.getAltLocs()) {
115-
Group nAltLocGroup = (Group)altLocGroup.clone();
116-
n.addAltLoc(nAltLocGroup);
117-
}
118-
}
119-
return n;
120-
}
96+
// no need to implement clone here, it's already in super class
12197
}

‎biojava-structure/src/main/java/org/biojava/nbio/structure/io/PDBFileParser.java‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1836,6 +1836,7 @@ private void pdb_ATOM_Handler(String line) {
18361836
logger.warn("Element column was empty for atom {} {}. Assigning atom element "
18371837
+ "from Chemical Component Dictionary information", fullname.trim(), pdbnumber);
18381838
} else {
1839+
18391840
try {
18401841
element = Element.valueOfIgnoreCase(elementSymbol);
18411842
guessElement = false;
@@ -1863,7 +1864,13 @@ private void pdb_ATOM_Handler(String line) {
18631864
logger.warn("Atom name {} was not found in the Chemical Component Dictionary information of {}. "
18641865
+ "Assigning generic element R to it", fullname.trim(), currentGroup.getPDBName());
18651866
} else {
1866-
element = Element.valueOfIgnoreCase(elementSymbol);
1867+
try {
1868+
element = Element.valueOfIgnoreCase(elementSymbol);
1869+
} catch (IllegalArgumentException e) {
1870+
// this can still happen for cases like UNK
1871+
logger.warn("Element symbol {} found in chemical component dictionary for Atom {} {} could not be recognised as a known element. "
1872+
+ "Assigning generic element R to it", elementSymbol, fullname.trim(), pdbnumber);
1873+
}
18671874
}
18681875
} else {
18691876
logger.warn("Chemical Component Dictionary information was not found for Atom name {}. "

‎biojava-structure/src/main/java/org/biojava/nbio/structure/io/mmtf/MmtfUtils.java‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
package org.biojava.nbio.structure.io.mmtf;
22

3-
import java.io.FileNotFoundException;
43
import java.text.DateFormat;
54
import java.text.SimpleDateFormat;
65
import java.util.ArrayList;
@@ -42,13 +41,18 @@
4241
import org.biojava.nbio.structure.xtal.SpaceGroup;
4342
import org.rcsb.mmtf.dataholders.DsspType;
4443
import org.rcsb.mmtf.utils.CodecUtils;
44+
import org.slf4j.Logger;
45+
import org.slf4j.LoggerFactory;
4546

4647
/**
4748
* A utils class of functions needed for Biojava to read and write to mmtf.
4849
* @author Anthony Bradley
4950
*
5051
*/
5152
public class MmtfUtils {
53+
54+
private static final Logger LOGGER = LoggerFactory.getLogger(MmtfUtils.class);
55+
5256
/**
5357
* Set up the configuration parameters for BioJava.
5458
*/
@@ -139,13 +143,12 @@ public static void calculateDsspSecondaryStructure(Structure bioJavaStruct) {
139143
ssp.calculate(bioJavaStruct, true);
140144
}
141145
catch(StructureException e) {
142-
try{
146+
LOGGER.warn("Could not calculate secondary structure (error {}). Will try to get a DSSP file from the RCSB web server instead.", e.getMessage());
147+
148+
try {
143149
DSSPParser.fetch(bioJavaStruct.getPDBCode(), bioJavaStruct, true); //download from PDB the DSSP result
144-
}
145-
catch(FileNotFoundException enew){
146-
}
147-
catch(Exception bige){
148-
System.out.println(bige);
150+
} catch(Exception bige){
151+
LOGGER.warn("Could not get a DSSP file from RCSB web server. There will not be secondary structure assignment for this structure ({}). Error: {}", bioJavaStruct.getPDBCode(), bige.getMessage());
149152
}
150153
}
151154
}

‎biojava-structure/src/test/java/org/biojava/nbio/structure/TestCloning.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,12 @@ private void compareCloned(Structure s, Structure c) throws StructureException {
131131
//}
132132
assertEquals(g.getAltLocs().size(), testGroup.getAltLocs().size());
133133
}
134+
135+
it = test.getSeqResGroups().iterator();
136+
for (Group g: chain.getSeqResGroups()) {
137+
Group testGroup = it.next();
138+
assertEquals(g.getAltLocs().size(), testGroup.getAltLocs().size());
139+
}
134140
}
135141

136142
Atom[] allAtoms = StructureTools.getAllAtomArray(s);

‎biojava-structure/src/test/java/org/biojava/nbio/structure/io/mmtf/TestMmtfRoundTrip.java‎

Lines changed: 34 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
package org.biojava.nbio.structure.io.mmtf;
22

3-
import static org.junit.Assert.assertArrayEquals;
4-
import static org.junit.Assert.assertEquals;
5-
import static org.junit.Assert.assertTrue;
3+
import static org.junit.Assert.*;
64

75
import java.io.IOException;
86
import java.util.ArrayList;
@@ -82,6 +80,7 @@ private boolean checkIfAtomsSame(Structure structOne, Structure structTwo) {
8280
Chain chainTwo = chainsTwo.get(j);
8381
// Check they have the same chain id
8482
assertEquals(chainOne.getId(), chainTwo.getId());
83+
checkSeqresGroups(chainOne, chainTwo);
8584
List<Group> groupsOne = chainOne.getAtomGroups();
8685
List<Group> groupsTwo = chainTwo.getAtomGroups();
8786
if(groupsOne.size()!=groupsTwo.size()){
@@ -93,19 +92,20 @@ private boolean checkIfAtomsSame(Structure structOne, Structure structTwo) {
9392
Group groupOne = groupsOne.get(k);
9493
Group groupTwo = groupsTwo.get(k);
9594
// Check if the groups are of the same type
96-
if(groupOne.getType().equals(groupTwo.getType())==false){
95+
if(!groupOne.getType().equals(groupTwo.getType())){
9796
System.out.println("Error - diff group type: "+structOne.getPDBCode());
9897
System.out.println(groupOne.getPDBName() + " and type: "+groupOne.getType());
9998
System.out.println(groupTwo.getPDBName() + " and type: "+groupTwo.getType());;
10099
}
101-
// Check the single letter amino aicd is correct
100+
// Check the single letter amino acid is correct
102101
if(groupOne.getChemComp().getOne_letter_code().length()==1 && groupTwo.getChemComp().getOne_letter_code().length()==1){
103102
if(!groupOne.getChemComp().getOne_letter_code().equals(groupTwo.getChemComp().getOne_letter_code())){
104103
System.out.println(groupOne.getPDBName());
105104
}
106105
assertEquals(groupOne.getChemComp().getOne_letter_code(), groupTwo.getChemComp().getOne_letter_code());
107106
}
108107
assertEquals(groupOne.getType(), groupTwo.getType());
108+
assertEquals(groupOne.getPDBName(), groupTwo.getPDBName());
109109
assertEquals(groupOne.getResidueNumber().getSeqNum(), groupTwo.getResidueNumber().getSeqNum());
110110
assertEquals(groupOne.getResidueNumber().getInsCode(), groupTwo.getResidueNumber().getInsCode());
111111
assertEquals(groupOne.getResidueNumber().getChainName(), groupTwo.getResidueNumber().getChainName());
@@ -142,15 +142,15 @@ private boolean checkIfAtomsSame(Structure structOne, Structure structTwo) {
142142
for(int l=0;l<atomsOne.size();l++){
143143
Atom atomOne = atomsOne.get(l);
144144
Atom atomTwo = atomsTwo.get(l);
145-
assertTrue(atomOne.getGroup().getPDBName().equals(atomTwo.getGroup().getPDBName()));
146-
assertTrue(atomOne.getCharge()==atomTwo.getCharge());
145+
assertEquals(atomOne.getGroup().getPDBName(), atomTwo.getGroup().getPDBName());
146+
assertEquals(atomOne.getCharge(), atomTwo.getCharge());
147147
// Check the coords are the same to three db
148148
assertArrayEquals(atomOne.getCoords(), atomTwo.getCoords(), 0.0009999999);
149149
assertEquals(atomOne.getTempFactor(), atomTwo.getTempFactor(), 0.009999999);
150150
assertEquals(atomOne.getOccupancy(), atomTwo.getOccupancy(), 0.009999999);
151-
assertTrue(atomOne.getElement()==atomTwo.getElement());
152-
assertTrue(atomOne.getName().equals(atomTwo.getName()));
153-
assertTrue(atomOne.getAltLoc()==atomTwo.getAltLoc());
151+
assertEquals(atomOne.getElement(), atomTwo.getElement());
152+
assertEquals(atomOne.getName(),atomTwo.getName());
153+
assertEquals(atomOne.getAltLoc(), atomTwo.getAltLoc());
154154
if(i==0){
155155
if(atomOne.getBonds()==null){
156156
if(atomTwo.getBonds()!=null){
@@ -246,4 +246,28 @@ public int compare(Chain o1, Chain o2) {
246246

247247
}
248248

249+
private void checkSeqresGroups(Chain chainOne, Chain chainTwo) {
250+
251+
assertEquals(chainOne.getSeqResGroups().size(), chainTwo.getSeqResGroups().size());
252+
253+
for (int i=0; i<chainOne.getSeqResGroups().size(); i++) {
254+
Group gOne = chainOne.getSeqResGroup(i);
255+
Group gTwo = chainTwo.getSeqResGroup(i);
256+
assertNotNull(gOne.getChemComp());
257+
assertNotNull(gTwo.getChemComp());
258+
assertEquals(gOne.getChemComp().getOne_letter_code(), gTwo.getChemComp().getOne_letter_code());
259+
260+
assertEquals(gOne.getResidueNumber(), gTwo.getResidueNumber());
261+
//assertEquals(gOne.getPDBName(), gTwo.getPDBName());
262+
}
263+
264+
// this is to test issue #517: a null pointer happens if the group hasn't been cloned (including the chem comp associated to it)
265+
Chain clonedChain = (Chain)chainTwo.clone();
266+
assertEquals(chainTwo.getSeqResGroups().size(), clonedChain.getSeqResGroups().size());
267+
for (int i=0; i<clonedChain.getSeqResGroups().size(); i++) {
268+
Group g = clonedChain.getSeqResGroup(i);
269+
assertNotNull(g.getChemComp());
270+
271+
}
272+
}
249273
}

0 commit comments

Comments
 (0)