From: gmungoc Date: Wed, 5 Oct 2016 15:17:59 +0000 (+0100) Subject: Merge branch 'develop' into task/JAL-2196pdbeProperties X-Git-Tag: Release_2_10_0b1~3^2~17 X-Git-Url: http://source.jalview.org/gitweb/?a=commitdiff_plain;h=e838644df5d5a10a16cf0ad7fb23d24dd7d2729a;hp=--cc;p=jalview.git Merge branch 'develop' into task/JAL-2196pdbeProperties Conflicts: src/jalview/datamodel/PDBEntry.java test/jalview/ws/dbsources/UniprotTest.java --- e838644df5d5a10a16cf0ad7fb23d24dd7d2729a diff --cc src/jalview/datamodel/PDBEntry.java index 7c2d290,1403595..6a6ccd0 --- a/src/jalview/datamodel/PDBEntry.java +++ b/src/jalview/datamodel/PDBEntry.java @@@ -22,20 -22,11 +22,21 @@@ package jalview.datamodel import jalview.util.CaseInsensitiveString; +import java.util.Collections; +import java.util.Enumeration; import java.util.Hashtable; public class PDBEntry { + + /** + * constant for storing chain code in properties table + */ + private static final String CHAIN_ID = "chain_code"; + - Hashtable properties; ++ private Hashtable properties; ++ + private static final int PDB_ID_LENGTH = 4; private String file; @@@ -76,11 -67,16 +77,10 @@@ } } - /** - * constant for storing chain code in properties table - */ - private static final String CHAIN_ID = "chain_code"; - - Hashtable properties; - /* - * (non-Javadoc) - * - * @see java.lang.Object#equals(java.lang.Object) + /** + * Answers true if obj is a PDBEntry with the same id and chain code (both + * ignoring case), file, type and properties */ @Override public boolean equals(Object obj) @@@ -125,8 -131,19 +135,19 @@@ public PDBEntry(String pdbId, String chain, PDBEntry.Type type, String filePath) { + init(pdbId, chain, type, filePath); + } + + /** + * @param pdbId + * @param chain - * @param type ++ * @param entryType + * @param filePath + */ - void init(String pdbId, String chain, PDBEntry.Type type, String filePath) ++ void init(String pdbId, String chain, PDBEntry.Type entryType, String filePath) + { this.id = pdbId; -- this.type = type == null ? null : type.toString(); ++ this.type = entryType == null ? null : entryType.toString(); this.file = filePath; setChainCode(chain); } @@@ -147,9 -164,38 +168,38 @@@ } } + /** + * Make a PDBEntry from a DBRefEntry. The accession code is used for the PDB + * id, but if it is 5 characters in length, the last character is removed and + * set as the chain code instead. + * + * @param dbr + */ + public PDBEntry(DBRefEntry dbr) + { + if (!DBRefSource.PDB.equals(dbr.getSource())) + { + throw new IllegalArgumentException("Invalid source: " + + dbr.getSource()); + } + + String pdbId = dbr.getAccessionId(); + String chainCode = null; + if (pdbId.length() == PDB_ID_LENGTH + 1) + { + char chain = pdbId.charAt(PDB_ID_LENGTH); + if (('a' <= chain && chain <= 'z') || ('A' <= chain && chain <= 'Z')) + { + pdbId = pdbId.substring(0, PDB_ID_LENGTH); + chainCode = String.valueOf(chain); + } + } + init(pdbId, chainCode, null, null); + } + - public void setFile(String file) + public void setFile(String f) { - this.file = file; + this.file = f; } public String getFile() @@@ -261,75 -275,109 +311,132 @@@ } /** + * Getter provided for Castor binding only. Application code should call + * getProperty() or getProperties() instead. + * + * @deprecated + * @see #getProperty(String) + * @see #getProperties() + * @see jalview.ws.dbsources.Uniprot#getUniprotEntries + * @return + */ + @Deprecated + public Hashtable getProps() + { + return properties; + } + + /** + * Setter provided for Castor binding only. Application code should call + * setProperty() instead. + * + * @deprecated + * @return + */ + @Deprecated + public void setProps(Hashtable props) + { + properties = props; + } + + /** - * update entry with details from another entry concerning the same PDB - * ID/file spec. + * Answers true if this object is either equivalent to, or can be 'improved' + * by, the given entry. + *

+ * If newEntry has the same id (ignoring case), and doesn't have a conflicting + * file spec or chain code, then update this entry from its file and/or chain + * code. * * @param newEntry * @return true if modifications were made */ - protected boolean updateFrom(PDBEntry newEntry) + public boolean updateFrom(PDBEntry newEntry) { - boolean modified = false; + if (this.equals(newEntry)) + { + return true; + } + + String newId = newEntry.getId(); + if (newId == null || getId() == null) + { + return false; // shouldn't happen + } + + /* - * id (less any chain code) has to match (ignoring case) ++ * id has to match (ignoring case) + */ + if (!getId().equalsIgnoreCase(newId)) + { + return false; + } + + /* + * Don't update if associated with different structure files + */ + String newFile = newEntry.getFile(); + if (newFile != null && getFile() != null && !newFile.equals(getFile())) + { + return false; + } - if (getFile() == null) + /* + * Don't update if associated with different chains (ignoring case) + */ + String newChain = newEntry.getChainCode(); + if (newChain != null && newChain.length() > 0 && getChainCode() != null + && getChainCode().length() > 0 + && !getChainCode().equalsIgnoreCase(newChain)) { - // update file and type of file - modified = newEntry.getFile() != null; - setFile(newEntry.getFile()); + return false; } - if (newEntry.getType() != null && newEntry.getFile() != null - && newEntry.getFile().equals(getFile())) + + /* + * set file path if not already set + */ + String newType = newEntry.getType(); + if (getFile() == null && newFile != null) + { + setFile(newFile); + setType(newType); + } + + /* + * set file type if new entry has it and we don't + * (for the case where file was not updated) + */ + if (getType() == null && newType != null) { - setType(newEntry.getType()); + setType(newType); } - if (getChainCode() == null - || (getChainCode() != null && getChainCode().length() == 0 && newEntry - .getChainCode() != null)) + + /* + * set chain if not already set (we excluded differing + * chains earlier) (ignoring case change only) + */ + if (newChain != null && newChain.length() > 0 + && !newChain.equalsIgnoreCase(getChainCode())) { - modified |= (getChainCode() == null || !newEntry.getChainCode() - .equals(getChainCode())); - setChainCode(newEntry.getChainCode()); + setChainCode(newChain); } - if (newEntry.properties != null) + + /* - * copy any new properties; notice this may include chain_code, - * but we excluded differing chain codes earlier ++ * copy any new or modified properties + */ - if (newEntry.getProperty() != null) ++ Enumeration newProps = newEntry.getProperties(); ++ while (newProps.hasMoreElements()) { - for (Entry entry : newEntry.properties.entrySet()) - if (properties == null) ++ /* ++ * copy properties unless value matches; this defends against changing ++ * the case of chain_code which is wrapped in a CaseInsensitiveString ++ */ ++ String key = newProps.nextElement(); ++ Object value = newEntry.getProperty(key); ++ if (!value.equals(getProperty(key))) { - if (!entry.getValue().equals(getProperty(entry.getKey()))) - { - modified = true; - } - setProperty(entry.getKey(), entry.getValue()); - properties = new Hashtable(); - } - for (Object p : newEntry.getProperty().keySet()) - { - /* - * copy properties unless value matches; this defends against changing - * the case of chain_code which is wrapped in a CaseInsensitiveString - */ - Object value = newEntry.getProperty().get(p); - if (!value.equals(properties.get(p))) - { - properties.put(p, newEntry.getProperty().get(p)); - } ++ setProperty(key, value); } } - return modified; + return true; } } diff --cc src/jalview/util/DBRefUtils.java index 4a0a55d,d43f5bc..e6aa472 --- a/src/jalview/util/DBRefUtils.java +++ b/src/jalview/util/DBRefUtils.java @@@ -26,8 -26,10 +26,9 @@@ import jalview.datamodel.PDBEntry import jalview.datamodel.SequenceI; import java.util.ArrayList; + import java.util.Arrays; import java.util.HashMap; import java.util.HashSet; -import java.util.Hashtable; import java.util.List; import java.util.Map; import java.util.Set; diff --cc src/jalview/ws/dbsources/Uniprot.java index 81b4caf,0c2af3b..caed598 --- a/src/jalview/ws/dbsources/Uniprot.java +++ b/src/jalview/ws/dbsources/Uniprot.java @@@ -222,6 -222,40 +222,38 @@@ public class Uniprot extends DbSourcePr { onlyPdbEntries.addElement(pdb); } + if ("EMBL".equals(pdb.getType())) + { + // look for a CDS reference and add it, too. - String cdsId = (String) pdb.getProperty() - .get("protein sequence ID"); ++ String cdsId = (String) pdb.getProperty("protein sequence ID"); + if (cdsId != null && cdsId.trim().length() > 0) + { + // remove version + String[] vrs = cdsId.split("\\."); + dbr = new DBRefEntry(DBRefSource.EMBLCDS, vrs.length > 1 ? vrs[1] + : DBRefSource.UNIPROT + ":" + dbVersion, vrs[0]); + dbRefs.add(dbr); + } + } + if ("Ensembl".equals(pdb.getType())) + { + /*UniprotXML + * + * + * + * + * + */ - String cdsId = (String) pdb.getProperty() - .get("protein sequence ID"); ++ String cdsId = (String) pdb.getProperty("protein sequence ID"); + if (cdsId != null && cdsId.trim().length() > 0) + { + dbr = new DBRefEntry(DBRefSource.ENSEMBL, DBRefSource.UNIPROT + + ":" + dbVersion, cdsId.trim()); + dbRefs.add(dbr); + + } + } + } sequence.setPDBId(onlyPdbEntries); diff --cc test/jalview/datamodel/PDBEntryTest.java index df29437,979fee4..e9d5cb2 --- a/test/jalview/datamodel/PDBEntryTest.java +++ b/test/jalview/datamodel/PDBEntryTest.java @@@ -23,8 -24,16 +24,14 @@@ package jalview.datamodel import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertFalse; import static org.testng.Assert.assertNotEquals; + import static org.testng.Assert.assertNotSame; import static org.testng.Assert.assertNull; + import static org.testng.Assert.assertSame; + import static org.testng.Assert.assertTrue; + import static org.testng.Assert.fail; + + import jalview.datamodel.PDBEntry.Type; -import java.util.Hashtable; - //import org.testng.Assert; import org.testng.annotations.AfterMethod; import org.testng.annotations.BeforeMethod; @@@ -76,8 -96,17 +94,17 @@@ public class PDBEntryTes assertNotEquals(case5, pdbEntry); assertNotEquals(case6, pdbEntry); assertNotEquals(case7, pdbEntry); - assertEquals(case8, pdbEntry); + assertNotEquals(case8, pdbEntry); assertEquals(case7, case9); + assertNotEquals(case9, case10); + + // add properties - case7.getProperty().put("hello", "world"); ++ case7.setProperty("hello", "world"); + assertNotEquals(case7, case9); - case9.getProperty().put("hello", "world"); ++ case9.setProperty("hello", "world"); + assertEquals(case7, case9); - case9.getProperty().put("hello", "WORLD"); ++ case9.setProperty("hello", "WORLD"); + assertNotEquals(case7, case9); /* * change string wrapper property to string... @@@ -100,4 -129,180 +127,175 @@@ pdbEntry.setChainCode(null); assertNull(pdbEntry.getChainCode()); } + + @Test(groups = { "Functional" }) + public void testGetType() + { + assertSame(PDBEntry.Type.FILE, PDBEntry.Type.getType("FILE")); + assertSame(PDBEntry.Type.FILE, PDBEntry.Type.getType("File")); + assertSame(PDBEntry.Type.FILE, PDBEntry.Type.getType("file")); + assertNotSame(PDBEntry.Type.FILE, PDBEntry.Type.getType("file ")); + } + + @Test(groups = { "Functional" }) + public void testTypeMatches() + { + // TODO Type.matches() is not used - delete? + assertTrue(PDBEntry.Type.FILE.matches("FILE")); + assertTrue(PDBEntry.Type.FILE.matches("File")); + assertTrue(PDBEntry.Type.FILE.matches("file")); + assertFalse(PDBEntry.Type.FILE.matches("FILE ")); + } + + @Test(groups = { "Functional" }) + public void testUpdateFrom() + { + PDBEntry pdb1 = new PDBEntry("3A6S", null, null, null); + PDBEntry pdb2 = new PDBEntry("3A6S", null, null, null); + assertTrue(pdb1.updateFrom(pdb2)); + + /* + * mismatch of pdb id not allowed + */ + pdb2 = new PDBEntry("1A70", "A", null, null); + assertFalse(pdb1.updateFrom(pdb2)); + assertNull(pdb1.getChainCode()); + + /* + * match of pdb id is not case sensitive + */ + pdb2 = new PDBEntry("3a6s", "A", null, null); + assertTrue(pdb1.updateFrom(pdb2)); + assertEquals(pdb1.getChainCode(), "A"); + assertEquals(pdb1.getId(), "3A6S"); + + /* + * add chain - with differing case for id + */ + pdb1 = new PDBEntry("3A6S", null, null, null); + pdb2 = new PDBEntry("3a6s", "A", null, null); + assertTrue(pdb1.updateFrom(pdb2)); + assertEquals(pdb1.getChainCode(), "A"); + + /* + * change of chain is not allowed + */ + pdb2 = new PDBEntry("3A6S", "B", null, null); + assertFalse(pdb1.updateFrom(pdb2)); + assertEquals(pdb1.getChainCode(), "A"); + + /* + * change chain from null + */ + pdb1 = new PDBEntry("3A6S", null, null, null); + pdb2 = new PDBEntry("3A6S", "B", null, null); + assertTrue(pdb1.updateFrom(pdb2)); + assertEquals(pdb1.getChainCode(), "B"); + + /* + * set file and type + */ + pdb2 = new PDBEntry("3A6S", "B", Type.FILE, "filePath"); + assertTrue(pdb1.updateFrom(pdb2)); + assertEquals(pdb1.getFile(), "filePath"); + assertEquals(pdb1.getType(), Type.FILE.toString()); + + /* + * change of file is not allowed + */ + pdb1 = new PDBEntry("3A6S", null, null, "file1"); + pdb2 = new PDBEntry("3A6S", "A", null, "file2"); + assertFalse(pdb1.updateFrom(pdb2)); + assertNull(pdb1.getChainCode()); + assertEquals(pdb1.getFile(), "file1"); + + /* + * set type without change of file + */ + pdb1 = new PDBEntry("3A6S", null, null, "file1"); + pdb2 = new PDBEntry("3A6S", null, Type.PDB, "file1"); + assertTrue(pdb1.updateFrom(pdb2)); + assertEquals(pdb1.getType(), Type.PDB.toString()); + + /* + * set file with differing case of id and chain code + */ + pdb1 = new PDBEntry("3A6S", "A", null, null); + pdb2 = new PDBEntry("3a6s", "a", Type.PDB, "file1"); + assertTrue(pdb1.updateFrom(pdb2)); + assertEquals(pdb1.getType(), Type.PDB.toString()); + assertEquals(pdb1.getId(), "3A6S"); // unchanged + assertEquals(pdb1.getFile(), "file1"); // updated + assertEquals(pdb1.getChainCode(), "A"); // unchanged + + /* + * changing nothing returns true + */ + pdb1 = new PDBEntry("3A6S", "A", Type.PDB, "file1"); + pdb2 = new PDBEntry("3A6S", null, null, null); + assertTrue(pdb1.updateFrom(pdb2)); + assertEquals(pdb1.getChainCode(), "A"); + assertEquals(pdb1.getType(), Type.PDB.toString()); + assertEquals(pdb1.getFile(), "file1"); + + /* + * add and update properties only + */ + pdb1 = new PDBEntry("3A6S", null, null, null); + pdb2 = new PDBEntry("3A6S", null, null, null); - // ughh properties not null if chain code has been set... - // JAL-2196 addresses this - pdb1.properties = new Hashtable(); - pdb2.properties = new Hashtable(); - pdb1.properties.put("destination", "mars"); - pdb1.properties.put("hello", "world"); - pdb2.properties.put("hello", "moon"); - pdb2.properties.put("goodbye", "world"); ++ pdb1.setProperty("destination", "mars"); ++ pdb1.setProperty("hello", "world"); ++ pdb2.setProperty("hello", "moon"); ++ pdb2.setProperty("goodbye", "world"); + assertTrue(pdb1.updateFrom(pdb2)); - assertEquals(pdb1.properties.get("destination"), "mars"); - assertEquals(pdb1.properties.get("hello"), "moon"); - assertEquals(pdb1.properties.get("goodbye"), "world"); ++ assertEquals(pdb1.getProperty("destination"), "mars"); ++ assertEquals(pdb1.getProperty("hello"), "moon"); ++ assertEquals(pdb1.getProperty("goodbye"), "world"); + + /* + * add properties only + */ + pdb1 = new PDBEntry("3A6S", null, null, null); + pdb2 = new PDBEntry("3A6S", null, null, null); - pdb2.properties = new Hashtable(); - pdb2.properties.put("hello", "moon"); ++ pdb2.setProperty("hello", "moon"); + assertTrue(pdb1.updateFrom(pdb2)); - assertEquals(pdb1.properties.get("hello"), "moon"); ++ assertEquals(pdb1.getProperty("hello"), "moon"); + } + + @Test(groups = { "Functional" }) + public void testConstructor_fromDbref() + { + PDBEntry pdb = new PDBEntry(new DBRefEntry("PDB", "0", "1A70")); + assertEquals(pdb.getId(), "1A70"); + assertNull(pdb.getChainCode()); + assertNull(pdb.getType()); + assertNull(pdb.getFile()); + + /* + * from dbref with chain code appended + */ + pdb = new PDBEntry(new DBRefEntry("PDB", "0", "1A70B")); + assertEquals(pdb.getId(), "1A70"); + assertEquals(pdb.getChainCode(), "B"); + + /* + * from dbref with overlong accession + */ + pdb = new PDBEntry(new DBRefEntry("PDB", "0", "1A70BC")); + assertEquals(pdb.getId(), "1A70BC"); + assertNull(pdb.getChainCode()); + + /* + * from dbref which is not for PDB + */ + try + { + pdb = new PDBEntry(new DBRefEntry("PDBe", "0", "1A70")); + fail("Expected exception"); + } catch (IllegalArgumentException e) + { + // expected; + } + } + } diff --cc test/jalview/ws/dbsources/UniprotTest.java index 26fcaf0,77f8078..4b88b2b --- a/test/jalview/ws/dbsources/UniprotTest.java +++ b/test/jalview/ws/dbsources/UniprotTest.java @@@ -21,7 -21,7 +21,8 @@@ package jalview.ws.dbsources; import static org.testng.AssertJUnit.assertEquals; +import static org.testng.AssertJUnit.assertFalse; + import static org.testng.AssertJUnit.assertNotNull; import static org.testng.AssertJUnit.assertNull; import jalview.datamodel.PDBEntry; @@@ -121,9 -124,30 +124,27 @@@ public class UniprotTes xref = xrefs.get(1); assertEquals("2FSR", xref.getId()); assertEquals("PDBsum", xref.getType()); - assertNull(xref.getProperty()); + assertFalse(xref.getProperties().hasMoreElements()); + + xref = xrefs.get(2); + assertEquals("AE007869", xref.getId()); + assertEquals("EMBL", xref.getType()); - assertNotNull(xref.getProperty()); + assertEquals("AAK85932.1", - (String) xref.getProperty().get("protein sequence ID")); ++ xref.getProperty("protein sequence ID")); + assertEquals("Genomic_DNA", - (String) xref.getProperty().get("molecule type")); - assertEquals(2, xref.getProperty().size()); - ++ xref.getProperty("molecule type")); } + @Test(groups = { "Functional" }) + public void testGetUniprotSequence() + { + UniprotEntry entry = new Uniprot().getUniprotEntries( + new StringReader(UNIPROT_XML)).get(0); + SequenceI seq = new Uniprot().uniprotEntryToSequenceI(entry); + assertNotNull(seq); + assertEquals(6, seq.getDBRefs().length); // 2*Uniprot, PDB, PDBsum, 2*EMBL + + } /** * Test the method that formats the sequence id */