From: gmungoc Date: Tue, 5 Sep 2017 11:48:47 +0000 (+0100) Subject: JAL-2225 fix bug in collating unique pdb files and their sequences X-Git-Tag: Release_2_10_3b1~47^2~3 X-Git-Url: http://source.jalview.org/gitweb/?p=jalview.git;a=commitdiff_plain;h=cfa72c2cceb390460b4f08c320146bd6910d8484 JAL-2225 fix bug in collating unique pdb files and their sequences --- diff --git a/src/jalview/gui/StructureChooser.java b/src/jalview/gui/StructureChooser.java index da10e3f..cf8c190 100644 --- a/src/jalview/gui/StructureChooser.java +++ b/src/jalview/gui/StructureChooser.java @@ -923,16 +923,10 @@ public class StructureChooser extends GStructureChooser } if (pdbEntriesToView.length > 1) { - ArrayList seqsMap = new ArrayList(); - for (SequenceI seq : sequences) - { - seqsMap.add(new SequenceI[] { seq }); - } - SequenceI[][] collatedSeqs = seqsMap.toArray(new SequenceI[0][0]); ssm.setProgressBar(null); ssm.setProgressBar(MessageManager.getString( "status.fetching_3d_structures_for_selected_entries")); - sViewer.viewStructures(pdbEntriesToView, collatedSeqs, alignPanel); + sViewer.viewStructures(pdbEntriesToView, sequences, alignPanel); } else { diff --git a/src/jalview/gui/StructureViewer.java b/src/jalview/gui/StructureViewer.java index e58b378..a1913c6 100644 --- a/src/jalview/gui/StructureViewer.java +++ b/src/jalview/gui/StructureViewer.java @@ -29,7 +29,11 @@ import jalview.structure.StructureSelectionManager; import java.awt.Rectangle; import java.util.ArrayList; +import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; +import java.util.Map; +import java.util.Map.Entry; /** * proxy for handling structure viewers. @@ -76,120 +80,155 @@ public class StructureViewer * @return */ public JalviewStructureDisplayI viewStructures(PDBEntry[] pdbs, - SequenceI[][] seqsForPdbs, AlignmentPanel ap) + SequenceI[] seqsForPdbs, AlignmentPanel ap) { JalviewStructureDisplayI viewer = onlyOnePdb(pdbs, seqsForPdbs, ap); if (viewer != null) { + /* + * user added structure to an existing viewer - all done + */ return viewer; } - return viewStructures(getViewerType(), pdbs, seqsForPdbs, ap); + + ViewerType viewerType = getViewerType(); + + // old way: + // PDBEntry[] pdbsForFile = getUniquePdbFiles(pdbs); + + // new way: + Map seqsForPdb = getSequencesForPdbs(pdbs, + seqsForPdbs); + PDBEntry[] pdbsForFile = seqsForPdb.keySet().toArray( + new PDBEntry[seqsForPdb.size()]); + SequenceI[][] theSeqs = seqsForPdb.values().toArray( + new SequenceI[seqsForPdb.size()][]); + JalviewStructureDisplayI sview = null; + if (viewerType.equals(ViewerType.JMOL)) + { + sview = new AppJmol(ap, pdbsForFile, theSeqs); + // ap.av.collateForPDB(pdbsForFile)); + } + else if (viewerType.equals(ViewerType.CHIMERA)) + { + sview = new ChimeraViewFrame(pdbsForFile, theSeqs, ap); + // ap.av.collateForPDB(pdbsForFile), ap); + } + else + { + Cache.log.error("Unknown structure viewer type " + + getViewerType().toString()); + } + return sview; } /** - * A strictly temporary method pending JAL-1761 refactoring. Determines if all - * the passed PDB entries are the same (this is the case if selected sequences - * to view structure for are chains of the same structure). If so, calls the - * single-pdb version of viewStructures and returns the viewer, else returns - * null. + * Converts the list of selected PDB entries (possibly including duplicates + * for multiple chains), and corresponding sequences, into a map of sequences + * for each distinct PDB file. Returns null if either argument is null, or + * their lengths do not match. * * @param pdbs - * @param seqsForPdbs - * @param ap + * @param seqs * @return */ - private JalviewStructureDisplayI onlyOnePdb(PDBEntry[] pdbs, - SequenceI[][] seqsForPdbs, AlignmentPanel ap) + static Map getSequencesForPdbs(PDBEntry[] pdbs, + SequenceI[] seqs) { - List seqs = new ArrayList(); - if (pdbs == null || pdbs.length == 0) + if (pdbs == null || seqs == null || pdbs.length != seqs.length) { return null; } - int i = 0; - String firstFile = pdbs[0].getFile(); - for (PDBEntry pdb : pdbs) + + /* + * we want only one 'representative' PDBEntry per distinct file name + * (there may be entries for distinct chains) + */ + Map pdbsSeen = new HashMap<>(); + + /* + * LinkedHashMap preserves order of PDB entries (significant if they + * will get superimposed to the first structure) + */ + Map> pdbSeqs = new LinkedHashMap<>(); + for (int i = 0; i < pdbs.length; i++) { + PDBEntry pdb = pdbs[i]; + SequenceI seq = seqs[i]; String pdbFile = pdb.getFile(); - if (pdbFile == null || !pdbFile.equals(firstFile)) + if (!pdbsSeen.containsKey(pdbFile)) { - return null; + pdbsSeen.put(pdbFile, pdb); + pdbSeqs.put(pdb, new ArrayList()); } - SequenceI[] pdbseqs = seqsForPdbs[i++]; - if (pdbseqs != null) + else { - for (SequenceI sq : pdbseqs) - { - seqs.add(sq); - } + pdb = pdbsSeen.get(pdbFile); + } + List seqsForPdb = pdbSeqs.get(pdb); + if (!seqsForPdb.contains(seq)) + { + seqsForPdb.add(seq); } } - return viewStructures(pdbs[0], seqs.toArray(new SequenceI[seqs.size()]), - ap); - } - public JalviewStructureDisplayI viewStructures(PDBEntry pdb, - SequenceI[] seqsForPdb, AlignmentPanel ap) - { - return viewStructures(getViewerType(), pdb, seqsForPdb, ap); - } - - protected JalviewStructureDisplayI viewStructures(ViewerType viewerType, - PDBEntry[] pdbs, SequenceI[][] seqsForPdbs, AlignmentPanel ap) - { - PDBEntry[] pdbsForFile = getUniquePdbFiles(pdbs); - JalviewStructureDisplayI sview = null; - if (viewerType.equals(ViewerType.JMOL)) + /* + * convert to Map + */ + Map result = new LinkedHashMap<>(); + for (Entry> entry : pdbSeqs.entrySet()) { - sview = new AppJmol(ap, pdbsForFile, - ap.av.collateForPDB(pdbsForFile)); + List theSeqs = entry.getValue(); + result.put(entry.getKey(), + theSeqs.toArray(new SequenceI[theSeqs.size()])); } - else if (viewerType.equals(ViewerType.CHIMERA)) - { - sview = new ChimeraViewFrame(pdbsForFile, - ap.av.collateForPDB(pdbsForFile), ap); - } - else - { - Cache.log.error("Unknown structure viewer type " - + getViewerType().toString()); - } - return sview; + + return result; } /** - * Convert the array of PDBEntry into an array with no filename repeated + * A strictly temporary method pending JAL-1761 refactoring. Determines if all + * the passed PDB entries are the same (this is the case if selected sequences + * to view structure for are chains of the same structure). If so, calls the + * single-pdb version of viewStructures and returns the viewer, else returns + * null. * * @param pdbs + * @param seqsForPdbs + * @param ap * @return */ - static PDBEntry[] getUniquePdbFiles(PDBEntry[] pdbs) + private JalviewStructureDisplayI onlyOnePdb(PDBEntry[] pdbs, + SequenceI[] seqsForPdbs, AlignmentPanel ap) { - if (pdbs == null) + List seqs = new ArrayList(); + if (pdbs == null || pdbs.length == 0) { return null; } - List uniques = new ArrayList(); - List filesSeen = new ArrayList(); - for (PDBEntry entry : pdbs) + int i = 0; + String firstFile = pdbs[0].getFile(); + for (PDBEntry pdb : pdbs) { - String file = entry.getFile(); - if (file == null) + String pdbFile = pdb.getFile(); + if (pdbFile == null || !pdbFile.equals(firstFile)) { - uniques.add(entry); + return null; } - else if (!filesSeen.contains(file)) + SequenceI pdbseq = seqsForPdbs[i++]; + if (pdbseq != null) { - uniques.add(entry); - filesSeen.add(file); + seqs.add(pdbseq); } } - return uniques.toArray(new PDBEntry[uniques.size()]); + return viewStructures(pdbs[0], seqs.toArray(new SequenceI[seqs.size()]), + ap); } - protected JalviewStructureDisplayI viewStructures(ViewerType viewerType, - PDBEntry pdb, SequenceI[] seqsForPdb, AlignmentPanel ap) + public JalviewStructureDisplayI viewStructures(PDBEntry pdb, + SequenceI[] seqsForPdb, AlignmentPanel ap) { + ViewerType viewerType = getViewerType(); JalviewStructureDisplayI sview = null; if (viewerType.equals(ViewerType.JMOL)) { diff --git a/test/jalview/gui/StructureViewerTest.java b/test/jalview/gui/StructureViewerTest.java index c1c1d5c..335863b 100644 --- a/test/jalview/gui/StructureViewerTest.java +++ b/test/jalview/gui/StructureViewerTest.java @@ -1,10 +1,17 @@ package jalview.gui; import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertFalse; import static org.testng.Assert.assertNull; +import static org.testng.Assert.assertSame; +import static org.testng.Assert.assertTrue; import jalview.datamodel.PDBEntry; import jalview.datamodel.PDBEntry.Type; +import jalview.datamodel.Sequence; +import jalview.datamodel.SequenceI; + +import java.util.Map; import org.testng.annotations.BeforeClass; import org.testng.annotations.Test; @@ -20,9 +27,9 @@ public class StructureViewerTest } @Test(groups = "Functional") - public void testGetUniquePdbFiles() + public void testGetSequencesForPdbs() { - assertNull(StructureViewer.getUniquePdbFiles(null)); + assertNull(StructureViewer.getSequencesForPdbs(null, null)); PDBEntry pdbe1 = new PDBEntry("1A70", "A", Type.PDB, "path1"); PDBEntry pdbe2 = new PDBEntry("3A6S", "A", Type.PDB, "path2"); @@ -30,13 +37,46 @@ public class StructureViewerTest 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); + PDBEntry[] pdbs = new PDBEntry[] { pdbe1, pdbe2, pdbe3, pdbe4, pdbe5, + pdbe6 }; + + /* + * seq1 ... seq6 associated with pdbe1 ... pdbe6 + */ + SequenceI[] seqs = new SequenceI[pdbs.length]; + for (int i = 0; i < seqs.length; i++) + { + seqs[i] = new Sequence("Seq" + i, "abc"); + } /* - * pdbe2 and pdbe5 get removed as having a duplicate file path + * pdbe3/5/6 should 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 }); + Map uniques = StructureViewer + .getSequencesForPdbs(pdbs, seqs); + assertTrue(uniques.containsKey(pdbe1)); + assertTrue(uniques.containsKey(pdbe2)); + assertFalse(uniques.containsKey(pdbe3)); + assertTrue(uniques.containsKey(pdbe4)); + assertFalse(uniques.containsKey(pdbe5)); + assertFalse(uniques.containsKey(pdbe6)); + + // 1A70 associates with seq1 and seq3 + SequenceI[] ss = uniques.get(pdbe1); + assertEquals(ss.length, 2); + assertSame(seqs[0], ss[0]); + assertSame(seqs[2], ss[1]); + + // 3A6S has seq2 and seq5 + ss = uniques.get(pdbe2); + assertEquals(ss.length, 2); + assertSame(seqs[1], ss[0]); + assertSame(seqs[4], ss[1]); + + // 1GAQ has seq4 and seq6 + ss = uniques.get(pdbe4); + assertEquals(ss.length, 2); + assertSame(seqs[3], ss[0]); + assertSame(seqs[5], ss[1]); } }