biojava · andreasprlic · Jul 28, 2016 · Jul 26, 2016 · Jul 27, 2016 · Jul 27, 2016
diff --git a/biojava-integrationtest/src/test/resources/1tap.pdb b/biojava-integrationtest/src/test/resources/1tap.pdb
diff --git a/biojava-structure/src/main/java/org/biojava/nbio/structure/AminoAcidImpl.java b/biojava-structure/src/main/java/org/biojava/nbio/structure/AminoAcidImpl.java
@@ -183,6 +183,10 @@ public Object clone() {
 				n.addAltLoc(nAltLocGroup);
 			}
 		}
+
+		if (chemComp!=null)
+			n.setChemComp(chemComp);
+
 
 		return n;
 	}

diff --git a/biojava-structure/src/main/java/org/biojava/nbio/structure/HetatomImpl.java b/biojava-structure/src/main/java/org/biojava/nbio/structure/HetatomImpl.java
@@ -90,7 +90,7 @@ public static enum PerformanceBehavior {
 
 	private Map<String,Atom> atomNameLookup;
 
-	private ChemComp chemComp ;
+	protected ChemComp chemComp ;
 
 	private List<Group> altLocs;
 
@@ -403,6 +403,9 @@ public Object clone() {
 				n.addAltLoc(nAltLocGroup);
 			}
 		}
+
+		if (chemComp!=null)
+			n.setChemComp(chemComp);
 
 		return n;
 	}

diff --git a/biojava-structure/src/main/java/org/biojava/nbio/structure/NucleotideImpl.java b/biojava-structure/src/main/java/org/biojava/nbio/structure/NucleotideImpl.java
@@ -93,29 +93,5 @@ public Atom getP() {
 
 	}
 
-	@Override
-	public Object clone(){
-		NucleotideImpl n = new NucleotideImpl();
-
-		n.setPDBFlag(has3D());
-		n.setResidueNumber(getResidueNumber());
-
-		n.setPDBName(getPDBName());
-
-		// copy the atoms
-		for (Atom atom1 : atoms) {
-			Atom atom = (Atom) atom1.clone();
-			n.addAtom(atom);
-			atom.setGroup(n);
-		}
-
-		// copying the alt loc groups if present, otherwise they stay null
-		if (getAltLocs()!=null && !getAltLocs().isEmpty()) {
-			for (Group altLocGroup:this.getAltLocs()) {
-				Group nAltLocGroup = (Group)altLocGroup.clone();
-				n.addAltLoc(nAltLocGroup);
-			}
-		}
-		return n;
-	}
+	// no need to implement clone here, it's already in super class
 }
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
@@ -1836,6 +1836,7 @@ private void  pdb_ATOM_Handler(String line)	{
 				logger.warn("Element column was empty for atom {} {}. Assigning atom element "
 						+ "from Chemical Component Dictionary information", fullname.trim(), pdbnumber);
 			} else {
+
 				try {
 					element = Element.valueOfIgnoreCase(elementSymbol);
 					guessElement = false;
@@ -1863,7 +1864,13 @@ private void  pdb_ATOM_Handler(String line)	{
 					logger.warn("Atom name {} was not found in the Chemical Component Dictionary information of {}. "
 							+ "Assigning generic element R to it", fullname.trim(), currentGroup.getPDBName());
 				} else {
-					element = Element.valueOfIgnoreCase(elementSymbol);
+					try {
+						element = Element.valueOfIgnoreCase(elementSymbol);
+					} catch (IllegalArgumentException e) {
+						// this can still happen for cases like UNK
+						logger.warn("Element symbol {} found in chemical component dictionary for Atom {} {} could not be recognised as a known element. "
+								+ "Assigning generic element R to it", elementSymbol, fullname.trim(), pdbnumber);
+					}
 				}			
 			} else {
 				logger.warn("Chemical Component Dictionary information was not found for Atom name {}. "

diff --git a/biojava-structure/src/main/java/org/biojava/nbio/structure/io/mmtf/MmtfUtils.java b/biojava-structure/src/main/java/org/biojava/nbio/structure/io/mmtf/MmtfUtils.java
@@ -1,6 +1,5 @@
 package org.biojava.nbio.structure.io.mmtf;
 
-import java.io.FileNotFoundException;
 import java.text.DateFormat;
 import java.text.SimpleDateFormat;
 import java.util.ArrayList;
@@ -42,13 +41,18 @@
 import org.biojava.nbio.structure.xtal.SpaceGroup;
 import org.rcsb.mmtf.dataholders.DsspType;
 import org.rcsb.mmtf.utils.CodecUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * A utils class of functions needed for Biojava to read and write to mmtf.
  * @author Anthony Bradley
  *
  */
 public class MmtfUtils {
+
+	private static final Logger LOGGER = LoggerFactory.getLogger(MmtfUtils.class);
+
 	/**
 	 * Set up the configuration parameters for BioJava.
 	 */
@@ -139,13 +143,12 @@ public static void calculateDsspSecondaryStructure(Structure bioJavaStruct) {
 			ssp.calculate(bioJavaStruct, true);
 		}
 		catch(StructureException e) {
-			try{
+			LOGGER.warn("Could not calculate secondary structure (error {}). Will try to get a DSSP file from the RCSB web server instead.", e.getMessage());
+
+			try {
 				DSSPParser.fetch(bioJavaStruct.getPDBCode(), bioJavaStruct, true); //download from PDB the DSSP result
-			}
-			catch(FileNotFoundException enew){
-			}
-			catch(Exception bige){
-				System.out.println(bige);
+			} catch(Exception bige){
+				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());
 			}
 		}
 	}

diff --git a/biojava-structure/src/test/java/org/biojava/nbio/structure/TestCloning.java b/biojava-structure/src/test/java/org/biojava/nbio/structure/TestCloning.java
@@ -131,6 +131,12 @@ private void compareCloned(Structure s, Structure c) throws StructureException {
 				//}
 				assertEquals(g.getAltLocs().size(), testGroup.getAltLocs().size());
 			}
+
+			it = test.getSeqResGroups().iterator();
+			for (Group g: chain.getSeqResGroups()) {
+				Group testGroup = it.next();
+				assertEquals(g.getAltLocs().size(), testGroup.getAltLocs().size());
+			}
 		}
 
 		Atom[] allAtoms = StructureTools.getAllAtomArray(s);

diff --git a/biojava-structure/src/test/java/org/biojava/nbio/structure/io/mmtf/TestMmtfRoundTrip.java b/biojava-structure/src/test/java/org/biojava/nbio/structure/io/mmtf/TestMmtfRoundTrip.java
@@ -1,8 +1,6 @@
 package org.biojava.nbio.structure.io.mmtf;
 
-import static org.junit.Assert.assertArrayEquals;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.*;
 
 import java.io.IOException;
 import java.util.ArrayList;
@@ -82,6 +80,7 @@ private boolean checkIfAtomsSame(Structure structOne, Structure structTwo) {
 				Chain chainTwo = chainsTwo.get(j);
 				// Check they have the same chain id
 				assertEquals(chainOne.getId(), chainTwo.getId());
+				checkSeqresGroups(chainOne, chainTwo);
 				List<Group> groupsOne = chainOne.getAtomGroups();
 				List<Group> groupsTwo = chainTwo.getAtomGroups();
 				if(groupsOne.size()!=groupsTwo.size()){
@@ -93,19 +92,20 @@ private boolean checkIfAtomsSame(Structure structOne, Structure structTwo) {
 					Group groupOne = groupsOne.get(k);
 					Group groupTwo = groupsTwo.get(k);
 					// Check if the groups are of the same type
-					if(groupOne.getType().equals(groupTwo.getType())==false){
+					if(!groupOne.getType().equals(groupTwo.getType())){
 						System.out.println("Error - diff group type: "+structOne.getPDBCode());
 						System.out.println(groupOne.getPDBName() + " and type: "+groupOne.getType());
 						System.out.println(groupTwo.getPDBName() + " and type: "+groupTwo.getType());;
 					}
-					// Check the single letter amino aicd is correct
+					// Check the single letter amino acid is correct
 					if(groupOne.getChemComp().getOne_letter_code().length()==1 && groupTwo.getChemComp().getOne_letter_code().length()==1){
 						if(!groupOne.getChemComp().getOne_letter_code().equals(groupTwo.getChemComp().getOne_letter_code())){
 							System.out.println(groupOne.getPDBName());
 						}
 						assertEquals(groupOne.getChemComp().getOne_letter_code(), groupTwo.getChemComp().getOne_letter_code());
 					}
 					assertEquals(groupOne.getType(), groupTwo.getType());
+					assertEquals(groupOne.getPDBName(), groupTwo.getPDBName());
 					assertEquals(groupOne.getResidueNumber().getSeqNum(), groupTwo.getResidueNumber().getSeqNum());
 					assertEquals(groupOne.getResidueNumber().getInsCode(), groupTwo.getResidueNumber().getInsCode());
 					assertEquals(groupOne.getResidueNumber().getChainName(), groupTwo.getResidueNumber().getChainName());
@@ -142,15 +142,15 @@ private boolean checkIfAtomsSame(Structure structOne, Structure structTwo) {
 					for(int l=0;l<atomsOne.size();l++){
 						Atom atomOne = atomsOne.get(l);
 						Atom atomTwo = atomsTwo.get(l);
-						assertTrue(atomOne.getGroup().getPDBName().equals(atomTwo.getGroup().getPDBName()));
-						assertTrue(atomOne.getCharge()==atomTwo.getCharge());
+						assertEquals(atomOne.getGroup().getPDBName(), atomTwo.getGroup().getPDBName());
+						assertEquals(atomOne.getCharge(), atomTwo.getCharge());
 						// Check the coords are the same to three db
 						assertArrayEquals(atomOne.getCoords(), atomTwo.getCoords(), 0.0009999999);
 						assertEquals(atomOne.getTempFactor(), atomTwo.getTempFactor(), 0.009999999);
 						assertEquals(atomOne.getOccupancy(), atomTwo.getOccupancy(), 0.009999999);
-						assertTrue(atomOne.getElement()==atomTwo.getElement());
-						assertTrue(atomOne.getName().equals(atomTwo.getName()));
-						assertTrue(atomOne.getAltLoc()==atomTwo.getAltLoc());
+						assertEquals(atomOne.getElement(), atomTwo.getElement());
+						assertEquals(atomOne.getName(),atomTwo.getName());
+						assertEquals(atomOne.getAltLoc(), atomTwo.getAltLoc());
 						if(i==0){
 							if(atomOne.getBonds()==null){
 								if(atomTwo.getBonds()!=null){
@@ -246,4 +246,28 @@ public int compare(Chain o1, Chain o2) {
 
 	}
 
+	private void checkSeqresGroups(Chain chainOne, Chain chainTwo) {
+
+		assertEquals(chainOne.getSeqResGroups().size(), chainTwo.getSeqResGroups().size());
+
+		for (int i=0; i<chainOne.getSeqResGroups().size(); i++) {
+			Group gOne = chainOne.getSeqResGroup(i);
+			Group gTwo = chainTwo.getSeqResGroup(i);
+			assertNotNull(gOne.getChemComp());
+			assertNotNull(gTwo.getChemComp());
+			assertEquals(gOne.getChemComp().getOne_letter_code(), gTwo.getChemComp().getOne_letter_code());
+
+			assertEquals(gOne.getResidueNumber(), gTwo.getResidueNumber());
+			//assertEquals(gOne.getPDBName(), gTwo.getPDBName());
+		}
+
+		// this is to test issue #517: a null pointer happens if the group hasn't been cloned (including the chem comp associated to it)
+		Chain clonedChain = (Chain)chainTwo.clone();
+		assertEquals(chainTwo.getSeqResGroups().size(), clonedChain.getSeqResGroups().size());
+		for (int i=0; i<clonedChain.getSeqResGroups().size(); i++) {
+			Group g = clonedChain.getSeqResGroup(i);
+			assertNotNull(g.getChemComp());
+
+		}
+	}
 }