Finally fixing #234.

josemduarte · josemduarte · commit 903891e6336a · 2015-06-09T18:53:39.000+02:00
This includes an important bug fix for mmCIF parser: quoted strings
appearing at the end of a line with no trailing spaces were not parsed
correctly (the last quote character was included in the string).
Trailing spaces are not a requirement of the CIF format, thus the parser
needs to really work in those cases.
diff --git a/biojava-structure/src/main/java/org/biojava/nbio/structure/io/mmcif/SimpleMMcifConsumer.java b/biojava-structure/src/main/java/org/biojava/nbio/structure/io/mmcif/SimpleMMcifConsumer.java
@@ -71,8 +71,19 @@ public class SimpleMMcifConsumer implements MMcifConsumer {
 	private List<StructConn> structConn;
 	private List<StructNcsOper> structNcsOper;
 
+	/**
+	 * A map of asym ids (internal chain ids) to strand ids (author chain ids) 
+	 * extracted from pdbx_poly_seq_scheme/pdbx_non_poly_seq_scheme categories
+	 */
 	private Map<String,String> asymStrandId;
 	
+	/**
+	 * A map of asym ids (internal chain ids) to strand ids (author chain ids) 
+	 * extracted from the information in _atom_sites category. Will be used
+	 * if no mapping is found in pdbx_poly_seq_scheme/pdbx_non_poly_seq_scheme
+	 */
+	private Map<String,String> asymId2StrandIdFromAtomSites;
+	
 	private Map<String,String> asymId2entityId;
 
 	private String current_nmr_model ;
@@ -411,15 +422,15 @@ public void newAtomSite(AtomSite atom) {
 		if ( params.isParseCAOnly() ){
 			// yes , user wants to get CA only
 			// only parse CA atoms...
-			if (! (atom.getLabel_atom_id().equals("CA") && atom.getType_symbol().equals("C"))) {
+			if (! (atom.getLabel_atom_id().equals(StructureTools.CA_ATOM_NAME) && atom.getType_symbol().equals("C"))) {
 				//System.out.println("ignoring " + line);
 				//atomCount--;
 				return;
 			}
 		}
 
-
-
+		// filling the map in case there's no pdbx_poly_seq_scheme/pdbx_non_poly_seq_scheme in the file
+		asymId2StrandIdFromAtomSites.put(atom.getLabel_asym_id(), atom.getAuth_asym_id());
 
 		//see if chain_id is one of the previous chains ...
 
@@ -566,7 +577,8 @@ public void documentStart() {
 		entityChains  = new ArrayList<Chain>();
 		structAsyms   = new ArrayList<StructAsym>();
 		asymStrandId  = new HashMap<String, String>();
-		asymId2entityId = new HashMap<String,String>();
+		asymId2StrandIdFromAtomSites = new HashMap<String, String>();
+		asymId2entityId = new HashMap<String,String>();		
 		structOpers   = new ArrayList<PdbxStructOperList>();
 		strucAssemblies = new ArrayList<PdbxStructAssembly>();
 		strucAssemblyGens = new ArrayList<PdbxStructAssemblyGen>();
@@ -635,30 +647,25 @@ public void documentEnd() {
 		//TODO: add support for structure.setConnections(connects);
 		
 
-		// mismatching Author assigned chain IDS and PDB internal chain ids:
-		// fix the chain IDS in the current model:
-
-		Set<String> asymIds = asymStrandId.keySet();
 		
-		if (asymIds.isEmpty()) {
-			logger.warn("No asym ids mapping found in file (categories pdbx_poly_seq_scheme/pdbx_non_poly_seq_scheme). Will create fake asym ids");
-
-			if (structure.nrModels()==0) {
-				logger.error("We should have some models at this point, something is wrong! We'll have an empty structure");
-			} else {
-				for (Chain chain : structure.getModel(0)) {
-					asymStrandId.put(chain.getChainID(),chain.getChainID());
-				}
-			}
+		boolean noAsymStrandIdMappingPresent = false;
+		if (asymStrandId.isEmpty()) {
+			logger.warn("No pdbx_poly_seq_scheme/pdbx_non_poly_seq_scheme categories present. Will use chain id mapping from _atom_sites category");
+			
+			asymStrandId = asymId2StrandIdFromAtomSites;
+			noAsymStrandIdMappingPresent = true;
 		}
+		
+		// mismatching Author assigned chain IDS and PDB internal chain ids:
+		// fix the chain IDS in the current model:
 
 		for (int i =0; i< structure.nrModels() ; i++){
 			List<Chain> model = structure.getModel(i);
 
 			List<Chain> pdbChains = new ArrayList<Chain>();
 
 			for (Chain chain : model) {
-				for (String asym : asymIds) {
+				for (String asym : asymStrandId.keySet()) {
 					if ( chain.getChainID().equals(asym)){
 						String newChainId = asymStrandId.get(asym);
 						
@@ -703,7 +710,7 @@ public void documentEnd() {
 				if (entityId==null) {
 					// this can happen for instance if the cif file didn't have _struct_asym category at all
 					// and thus we have no asymId2entityId mapping at all
-					logger.warn("No entity id could be found for chain {}", chain.getInternalChainID());
+					logger.warn("No entity id could be found for chain {}", chain.getInternalChainID());					
 					continue;
 				}
 				int eId = Integer.parseInt(entityId);
@@ -736,7 +743,33 @@ public void documentEnd() {
 
 			}			
 			
+			if (noAsymStrandIdMappingPresent) {
+				// At this point we have to make sure that all chains are polymeric (possibly with some attached non-polymers)
+				// because that's the current biojava model. 
+				// It can happen that all molecules are assigned to their own chains, for instance in mmCIF files 
+				// produced by phenix (in that case there will be noAsymStrandIdMapping present (no pdbx_poly_seq_scheme))
+				// mmCIF files produced by the PDB follow the convention: distinct asym_id for every 
+				// molecule (poly or non-poly) whilst a single author_asym_id for polymer + its ligands
+				it = pdbChains.iterator();
+				while (it.hasNext()) {
+					Chain chain = it.next();
+					GroupType predominantGroupType = StructureTools.getPredominantGroupType(chain);
+					if (StructureTools.isChainWaterOnly(chain)) {
+						it.remove();
+						logger.warn("Chain with chain id {} (asym id {}) and {} residues, contains only waters. Will ignore the chain because it doesn't fit into the BioJava structure data model.",
+								chain.getChainID(),chain.getInternalChainID(),chain.getAtomGroups().size());
+					}
+					else if (predominantGroupType != GroupType.AMINOACID && 
+							 predominantGroupType!=GroupType.NUCLEOTIDE ) {
+						logger.warn("Chain with chain id {} (asym id {}) and {} residues, does not seem to be polymeric. Will ignore the chain because it doesn't fit into the BioJava structure data model.",
+								chain.getChainID(),chain.getInternalChainID(),chain.getAtomGroups().size());
+						it.remove();
+					}
+				}
+			}
 		}
+		
+		
 
 		// to make sure we have Compounds linked to chains, we call getCompounds() which will lazily initialise the
 		// compounds using heuristics (see CompoundFinder) in the case that they were not explicitly present in the file
diff --git a/biojava-structure/src/main/java/org/biojava/nbio/structure/io/mmcif/SimpleMMcifParser.java b/biojava-structure/src/main/java/org/biojava/nbio/structure/io/mmcif/SimpleMMcifParser.java
@@ -377,9 +377,9 @@ private List<String> processSingleLine(String line){
 			if (i < line.length() - 1)
 				nextC = line.charAt(i+1);
 			
-			Character lastC = null;
+			Character prevC = null;
 			if (i>0) 
-				lastC = line.charAt(i-1);
+				prevC = line.charAt(i-1);
 			
 			if  (c == ' ') {
 
@@ -398,12 +398,9 @@ private List<String> processSingleLine(String line){
 
 					boolean wordEnd = false;
 					if (! inS2) {
-						if (nextC != null){
-							//System.out.println("nextC: >"+nextC+"<");
-							if ( Character.isWhitespace(nextC)){
-								i++;
-								wordEnd = true;
-							}
+						if (nextC==null || Character.isWhitespace(nextC)){
+							i++;
+							wordEnd = true;
 						}
 					}
 
@@ -420,7 +417,7 @@ private List<String> processSingleLine(String line){
 						word += c;
 					}
 
-				} else if (lastC==null || lastC==' ') {
+				} else if (prevC==null || prevC==' ') {
 					// the beginning of a new string
 					inString = true;
 					inS1     = true;
@@ -432,12 +429,9 @@ private List<String> processSingleLine(String line){
 
 					boolean wordEnd = false;
 					if (! inS1) {
-						if (nextC != null){
-							//System.out.println("nextC: >"+nextC+"<");
-							if ( Character.isWhitespace(nextC)){
-								i++;
-								wordEnd = true;
-							}
+						if (nextC==null || Character.isWhitespace(nextC)){
+							i++;
+							wordEnd = true;
 						}
 					}
 
@@ -452,7 +446,7 @@ private List<String> processSingleLine(String line){
 					} else {
 						word += c;
 					}
-				}  else if (lastC==null || lastC==' ') {
+				}  else if (prevC==null || prevC==' ') {
 					// the beginning of a new string
 					inString = true;
 					inS2     = true;
diff --git a/biojava-structure/src/test/java/org/biojava/nbio/structure/io/TestNonDepositedFiles.java b/biojava-structure/src/test/java/org/biojava/nbio/structure/io/TestNonDepositedFiles.java
@@ -194,7 +194,7 @@ private void checkChains(Structure s) {
 	 * See github issue #234
 	 * @throws IOException
 	 */
-	//@Test
+	@Test
 	public void testPhenixFile() throws IOException {
 		InputStream inStream = new GZIPInputStream(this.getClass().getResourceAsStream("/org/biojava/nbio/structure/io/4lup_phenix_output.cif.gz"));
 		MMcifParser parser = new SimpleMMcifParser();
diff --git a/biojava-structure/src/test/resources/org/biojava/nbio/structure/io/difficult_mmcif_quoting.cif b/biojava-structure/src/test/resources/org/biojava/nbio/structure/io/difficult_mmcif_quoting.cif
@@ -74,6 +74,11 @@ J.Mol.Biol.                309 401 ? 2001 JMOBAK UK 0022-2836 0070 ? 11371161 10
 'Acta Crystallogr.,Sect.D' 56  137 ? 2000 ABCRE6 DK 0907-4449 0766 ? 10666593 10.1107/S0907444999015334      
 2       'Structure of the Capsid of Pf3 Filamentous Phage Determined from X-Ray Fibre Diffraction Data at 3.1 A Resolution'                                 
 J.Mol.Biol.                283 155 ? 1998 JMOBAK UK 0022-2836 0070 ? 9761681  10.1006/JMBI.1998.2081         
+#
+_space_group.name_H-M_alt         'P 1 21 1'
+_space_group.name_Hall            " P 2yb"
+_space_group.IT_number            4
+_space_group.crystal_system       monoclinic
 # 
 loop_
 _citation_author.citation_id 
@@ -83,4 +88,3 @@ primary 'Pederson, D.M.'   1
 primary "Welsh, L.C."      2  
 primary 'Marvin, D.A.'     3  
 
-
