From a5be53df96dabea00429121a742ae476872640eb Mon Sep 17 00:00:00 2001 From: gmungoc Date: Thu, 13 Oct 2016 10:55:42 +0100 Subject: [PATCH] JAL-2044 ensure AAStructureBindingModel.pdbEntry has no file duplicated; pull up modelFileNames --- src/jalview/ext/jmol/JalviewJmolBinding.java | 48 ++++++------ .../ext/rbvi/chimera/JalviewChimeraBinding.java | 5 -- src/jalview/gui/StructureViewer.java | 36 ++++++++- .../structures/models/AAStructureBindingModel.java | 19 ++++- test/jalview/gui/StructureViewerTest.java | 33 +++++++++ .../models/AAStructureBindingModelTest.java | 78 +++++++++++++------- 6 files changed, 164 insertions(+), 55 deletions(-) create mode 100644 test/jalview/gui/StructureViewerTest.java diff --git a/src/jalview/ext/jmol/JalviewJmolBinding.java b/src/jalview/ext/jmol/JalviewJmolBinding.java index a577559..fbac400 100644 --- a/src/jalview/ext/jmol/JalviewJmolBinding.java +++ b/src/jalview/ext/jmol/JalviewJmolBinding.java @@ -43,6 +43,7 @@ import java.awt.event.ComponentListener; import java.io.File; import java.net.URL; import java.security.AccessControlException; +import java.util.ArrayList; import java.util.Hashtable; import java.util.List; import java.util.Map; @@ -93,11 +94,6 @@ public abstract class JalviewJmolBinding extends AAStructureBindingModel boolean loadedInline; - /** - * current set of model filenames loaded in the Jmol instance - */ - String[] modelFileNames = null; - StringBuffer resetLastRes = new StringBuffer(); public Viewer viewer; @@ -258,8 +254,12 @@ public abstract class JalviewJmolBinding extends AAStructureBindingModel } catch (InterruptedException i) { } - ; } + + /* + * get the distinct structure files modelled + * (a file with multiple chains may map to multiple sequences) + */ String[] files = getPdbFile(); if (!waitForFileLoad(files)) { @@ -421,6 +421,8 @@ public abstract class JalviewJmolBinding extends AAStructureBindingModel } } StringBuilder command = new StringBuilder(256); + // command.append("set spinFps 10;\n"); + for (int pdbfnum = 0; pdbfnum < files.length; pdbfnum++) { if (pdbfnum == refStructure || selcom[pdbfnum] == null @@ -650,15 +652,15 @@ public abstract class JalviewJmolBinding extends AAStructureBindingModel } if (modelFileNames == null) { - String mset[] = new String[viewer.ms.mc]; - _modelFileNameMap = new int[mset.length]; + List mset = new ArrayList(); + _modelFileNameMap = new int[viewer.ms.mc]; String m = viewer.ms.getModelFileName(0); if (m != null) { - mset[0] = m; + String filePath = m; try { - mset[0] = new File(m).getAbsolutePath(); + filePath = new File(m).getAbsolutePath(); } catch (AccessControlException x) { // usually not allowed to do this in applet @@ -666,39 +668,43 @@ public abstract class JalviewJmolBinding extends AAStructureBindingModel .println("jmolBinding: Using local file string from Jmol: " + m); } - if (mset[0].indexOf("/file:") != -1) + if (filePath.indexOf("/file:") != -1) { // applet path with docroot - discard as format won't match pdbfile - mset[0] = m; + filePath = m; } + mset.add(filePath); _modelFileNameMap[0] = 0; // filename index for first model is always 0. } int j = 1; - for (int i = 1; i < mset.length; i++) + for (int i = 1; i < viewer.ms.mc; i++) { m = viewer.ms.getModelFileName(i); - mset[j] = m; + String filePath = m; if (m != null) { try { - mset[j] = new File(m).getAbsolutePath(); + filePath = new File(m).getAbsolutePath(); } catch (AccessControlException x) { // usually not allowed to do this in applet, so keep raw handle // System.err.println("jmolBinding: Using local file string from Jmol: "+m); } } - _modelFileNameMap[j] = i; // record the model index for the filename - // skip any additional models in the same file (NMR structures) - if ((mset[j] == null ? mset[j] != mset[j - 1] - : (mset[j - 1] == null || !mset[j].equals(mset[j - 1])))) + + /* + * add this model unless it is read from a structure file we have + * already seen (example: 2MJW is an NMR structure with 10 models) + */ + if (!mset.contains(filePath)) { + mset.add(filePath); + _modelFileNameMap[j] = i; // record the model index for the filename j++; } } - modelFileNames = new String[j]; - System.arraycopy(mset, 0, modelFileNames, 0, j); + modelFileNames = mset.toArray(new String[mset.size()]); } return modelFileNames; } diff --git a/src/jalview/ext/rbvi/chimera/JalviewChimeraBinding.java b/src/jalview/ext/rbvi/chimera/JalviewChimeraBinding.java index 944ef52..7ba9186 100644 --- a/src/jalview/ext/rbvi/chimera/JalviewChimeraBinding.java +++ b/src/jalview/ext/rbvi/chimera/JalviewChimeraBinding.java @@ -101,11 +101,6 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel private String lastCommand; - /* - * current set of model filenames loaded - */ - String[] modelFileNames = null; - String lastHighlightCommand; /* diff --git a/src/jalview/gui/StructureViewer.java b/src/jalview/gui/StructureViewer.java index 21b2984..189d490 100644 --- a/src/jalview/gui/StructureViewer.java +++ b/src/jalview/gui/StructureViewer.java @@ -136,14 +136,16 @@ public class StructureViewer protected JalviewStructureDisplayI viewStructures(ViewerType viewerType, PDBEntry[] pdbs, SequenceI[][] seqsForPdbs, AlignmentPanel ap) { + PDBEntry[] pdbsForFile = getUniquePdbFiles(pdbs); JalviewStructureDisplayI sview = null; if (viewerType.equals(ViewerType.JMOL)) { - sview = new AppJmol(ap, pdbs, ap.av.collateForPDB(pdbs)); + sview = new AppJmol(ap, pdbsForFile, ap.av.collateForPDB(pdbsForFile)); } else if (viewerType.equals(ViewerType.CHIMERA)) { - sview = new ChimeraViewFrame(pdbs, ap.av.collateForPDB(pdbs), ap); + sview = new ChimeraViewFrame(pdbsForFile, + ap.av.collateForPDB(pdbsForFile), ap); } else { @@ -153,6 +155,36 @@ public class StructureViewer return sview; } + /** + * Convert the array of PDBEntry into an array with no filename repeated + * + * @param pdbs + * @return + */ + static PDBEntry[] getUniquePdbFiles(PDBEntry[] pdbs) + { + if (pdbs == null) + { + return null; + } + List uniques = new ArrayList(); + List filesSeen = new ArrayList(); + for (PDBEntry entry : pdbs) + { + String file = entry.getFile(); + if (file == null) + { + uniques.add(entry); + } + else if (!filesSeen.contains(file)) + { + uniques.add(entry); + filesSeen.add(file); + } + } + return uniques.toArray(new PDBEntry[uniques.size()]); + } + protected JalviewStructureDisplayI viewStructures(ViewerType viewerType, PDBEntry pdb, SequenceI[] seqsForPdb, AlignmentPanel ap) { diff --git a/src/jalview/structures/models/AAStructureBindingModel.java b/src/jalview/structures/models/AAStructureBindingModel.java index dc42315..0a252fb 100644 --- a/src/jalview/structures/models/AAStructureBindingModel.java +++ b/src/jalview/structures/models/AAStructureBindingModel.java @@ -51,6 +51,10 @@ public abstract class AAStructureBindingModel extends private StructureSelectionManager ssm; + /* + * distinct PDB entries (pdb files) associated + * with sequences + */ private PDBEntry[] pdbEntry; /* @@ -75,6 +79,11 @@ public abstract class AAStructureBindingModel extends private boolean finishedInit = false; /** + * current set of model filenames loaded in the Jmol instance + */ + protected String[] modelFileNames = null; + + /** * Data bean class to simplify parameterisation in superposeStructures */ protected class SuperposeData @@ -521,6 +530,10 @@ public abstract class AAStructureBindingModel extends { int refStructure = -1; String[] files = getPdbFile(); + if (files == null) + { + return -1; + } for (int pdbfnum = 0; pdbfnum < files.length; pdbfnum++) { StructureMapping[] mappings = getSsm().getMapping(files[pdbfnum]); @@ -565,7 +578,11 @@ public abstract class AAStructureBindingModel extends } structures[pdbfnum].pdbId = mapping.getPdbId(); structures[pdbfnum].isRna = theSequence.getRNA() != null; - // move on to next pdb file + + /* + * move on to next pdb file (ignore sequences for other chains + * for the same structure) + */ s = seqCountForPdbFile; break; } diff --git a/test/jalview/gui/StructureViewerTest.java b/test/jalview/gui/StructureViewerTest.java new file mode 100644 index 0000000..f8e9133 --- /dev/null +++ b/test/jalview/gui/StructureViewerTest.java @@ -0,0 +1,33 @@ +package jalview.gui; + +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertNull; + +import jalview.datamodel.PDBEntry; +import jalview.datamodel.PDBEntry.Type; + +import org.testng.annotations.Test; + +public class StructureViewerTest +{ + @Test(groups = "Functional") + public void testGetUniquePdbFiles() + { + assertNull(StructureViewer.getUniquePdbFiles(null)); + + PDBEntry pdbe1 = new PDBEntry("1A70", "A", Type.PDB, "path1"); + PDBEntry pdbe2 = new PDBEntry("3A6S", "A", Type.PDB, "path2"); + PDBEntry pdbe3 = new PDBEntry("1A70", "B", Type.PDB, "path1"); + PDBEntry pdbe4 = new PDBEntry("1GAQ", "A", Type.PDB, null); + PDBEntry pdbe5 = new PDBEntry("3A6S", "B", Type.PDB, "path2"); + PDBEntry pdbe6 = new PDBEntry("1GAQ", "B", Type.PDB, null); + + /* + * pdbe2 and pdbe5 get removed as having a duplicate file path + */ + PDBEntry[] uniques = StructureViewer.getUniquePdbFiles(new PDBEntry[] { + pdbe1, pdbe2, pdbe3, pdbe4, pdbe5, pdbe6 }); + assertEquals(uniques, + new PDBEntry[] { pdbe1, pdbe2, pdbe4, pdbe6 }); + } +} diff --git a/test/jalview/structures/models/AAStructureBindingModelTest.java b/test/jalview/structures/models/AAStructureBindingModelTest.java index bb81992..0d00169 100644 --- a/test/jalview/structures/models/AAStructureBindingModelTest.java +++ b/test/jalview/structures/models/AAStructureBindingModelTest.java @@ -49,26 +49,36 @@ import org.testng.annotations.Test; */ public class AAStructureBindingModelTest { + /* + * Scenario: Jalview has 4 sequences, corresponding to 1YCS (chains A and B), 3A6S|B, 1OOT|A + */ private static final String PDB_1 = "HEADER COMPLEX (ANTI-ONCOGENE/ANKYRIN REPEATS) 30-SEP-96 1YCS \n" + "ATOM 2 CA VAL A 97 24.134 4.926 45.821 1.00 47.43 C \n" + "ATOM 9 CA PRO A 98 25.135 8.584 46.217 1.00 41.60 C \n" + "ATOM 16 CA SER A 99 28.243 9.596 44.271 1.00 39.63 C \n" + "ATOM 22 CA GLN A 100 31.488 10.133 46.156 1.00 35.60 C \n" - + "ATOM 31 CA LYS A 101 33.323 11.587 43.115 1.00 41.69 C \n"; + // artificial jump in residue numbering to prove it is correctly + // mapped: + + "ATOM 31 CA LYS A 102 33.323 11.587 43.115 1.00 41.69 C \n" + + "ATOM 1857 CA GLU B 374 9.193 -16.005 95.870 1.00 54.22 C \n" + + "ATOM 1866 CA ILE B 375 7.101 -14.921 92.847 1.00 46.82 C \n" + + "ATOM 1874 CA VAL B 376 10.251 -13.625 91.155 1.00 47.80 C \n" + + "ATOM 1881 CA LYS B 377 11.767 -17.068 91.763 1.00 50.21 C \n" + + "ATOM 1890 CA PHE B 378 8.665 -18.948 90.632 1.00 44.85 C \n"; private static final String PDB_2 = "HEADER HYDROLASE 09-SEP-09 3A6S \n" - + "ATOM 2 CA MET A 1 15.366 -11.648 24.854 1.00 32.05 C \n" - + "ATOM 10 CA LYS A 2 16.846 -9.215 22.340 1.00 25.68 C \n" - + "ATOM 19 CA LYS A 3 15.412 -6.335 20.343 1.00 19.42 C \n" - + "ATOM 28 CA LEU A 4 15.629 -5.719 16.616 1.00 15.49 C \n" - + "ATOM 36 CA GLN A 5 14.412 -2.295 15.567 1.00 12.19 C \n"; + + "ATOM 2 CA MET B 1 15.366 -11.648 24.854 1.00 32.05 C \n" + + "ATOM 10 CA LYS B 2 16.846 -9.215 22.340 1.00 25.68 C \n" + + "ATOM 19 CA LYS B 3 15.412 -6.335 20.343 1.00 19.42 C \n" + + "ATOM 28 CA LEU B 4 15.629 -5.719 16.616 1.00 15.49 C \n" + + "ATOM 36 CA GLN B 5 14.412 -2.295 15.567 1.00 12.19 C \n"; private static final String PDB_3 = "HEADER STRUCTURAL GENOMICS 04-MAR-03 1OOT \n" - + "ATOM 2 CA SER A 1 29.427 3.330 -6.578 1.00 32.50 C \n" - + "ATOM 8 CA PRO A 2 29.975 3.340 -2.797 1.00 17.62 C \n" - + "ATOM 16 CA ALYS A 3 26.958 3.024 -0.410 0.50 8.78 C \n" - + "ATOM 33 CA ALA A 4 26.790 4.320 3.172 1.00 11.98 C \n" - + "ATOM 39 CA AVAL A 5 24.424 3.853 6.106 0.50 13.83 C \n"; + + "ATOM 2 CA SER A 7 29.427 3.330 -6.578 1.00 32.50 C \n" + + "ATOM 8 CA PRO A 8 29.975 3.340 -2.797 1.00 17.62 C \n" + + "ATOM 16 CA ALYS A 9 26.958 3.024 -0.410 0.50 8.78 C \n" + + "ATOM 33 CA ALA A 10 26.790 4.320 3.172 1.00 11.98 C \n" + + "ATOM 39 CA AVAL A 12 24.424 3.853 6.106 0.50 13.83 C \n"; AAStructureBindingModel testee; @@ -80,24 +90,28 @@ public class AAStructureBindingModelTest @BeforeMethod(alwaysRun = true) public void setUp() { - SequenceI seq1 = new Sequence("1YCS", "-VPSQK"); + SequenceI seq1a = new Sequence("1YCS|A", "-VPSQK"); + SequenceI seq1b = new Sequence("1YCS|B", "EIVKF-"); SequenceI seq2 = new Sequence("3A6S", "MK-KLQ"); SequenceI seq3 = new Sequence("1OOT", "SPK-AV"); - al = new Alignment(new SequenceI[] { seq1, seq2, seq3 }); + al = new Alignment(new SequenceI[] { seq1a, seq1b, seq2, seq3 }); al.setDataset(null); + /* + * give pdb files the name generated by Jalview for PASTE source + */ PDBEntry[] pdbFiles = new PDBEntry[3]; - pdbFiles[0] = new PDBEntry("1YCS", "A", Type.PDB, "1YCS.pdb"); - pdbFiles[1] = new PDBEntry("3A6S", "B", Type.PDB, "3A6S.pdb"); - pdbFiles[2] = new PDBEntry("1OOT", "A", Type.PDB, "1OOT.pdb"); + pdbFiles[0] = new PDBEntry("1YCS", "A", Type.PDB, "INLINE1YCS"); + pdbFiles[1] = new PDBEntry("3A6S", "B", Type.PDB, "INLINE3A6S"); + pdbFiles[2] = new PDBEntry("1OOT", "A", Type.PDB, "INLINE1OOT"); String[][] chains = new String[3][]; SequenceI[][] seqs = new SequenceI[3][]; - seqs[0] = new SequenceI[] { seq1 }; + seqs[0] = new SequenceI[] { seq1a, seq1b }; seqs[1] = new SequenceI[] { seq2 }; seqs[2] = new SequenceI[] { seq3 }; StructureSelectionManager ssm = new StructureSelectionManager(); - ssm.setMapping(new SequenceI[] { seq1 }, null, PDB_1, + ssm.setMapping(new SequenceI[] { seq1a, seq1b }, null, PDB_1, AppletFormatAdapter.PASTE); ssm.setMapping(new SequenceI[] { seq2 }, null, PDB_2, AppletFormatAdapter.PASTE); @@ -109,10 +123,6 @@ public class AAStructureBindingModelTest @Override public String[] getPdbFile() { - /* - * fudge 'filenames' to match those generated when PDBFile parses PASTE - * data - */ return new String[] { "INLINE1YCS", "INLINE3A6S", "INLINE1OOT" }; } @@ -140,7 +150,10 @@ public class AAStructureBindingModelTest @Test(groups = { "Functional" }) public void testFindSuperposableResidues() { - SuperposeData[] structs = new SuperposeData[al.getHeight()]; + /* + * create a data bean to hold data per structure file + */ + SuperposeData[] structs = new SuperposeData[testee.getPdbFile().length]; for (int i = 0; i < structs.length; i++) { structs[i] = testee.new SuperposeData(al.getWidth()); @@ -162,10 +175,23 @@ public class AAStructureBindingModelTest */ assertFalse(matched[0]); // gap in first sequence assertTrue(matched[1]); - assertFalse(matched[2]); // gap in second sequence - assertFalse(matched[3]); // gap in third sequence + assertFalse(matched[2]); // gap in third sequence + assertFalse(matched[3]); // gap in fourth sequence assertTrue(matched[4]); - assertTrue(matched[5]); + assertTrue(matched[5]); // gap in second sequence + + assertEquals("1YCS", structs[0].pdbId); + assertEquals("3A6S", structs[1].pdbId); + assertEquals("1OOT", structs[2].pdbId); + assertEquals("A", structs[0].chain); // ? struct has chains A _and_ B + assertEquals("B", structs[1].chain); + assertEquals("A", structs[2].chain); + // the 0's for unsuperposable positions propagate down the columns: + assertEquals("[0, 97, 98, 99, 100, 102]", + Arrays.toString(structs[0].pdbResNo)); + assertEquals("[0, 2, 0, 3, 4, 5]", Arrays.toString(structs[1].pdbResNo)); + assertEquals("[0, 8, 0, 0, 10, 12]", + Arrays.toString(structs[2].pdbResNo)); } @Test(groups = { "Functional" }) -- 1.7.10.2