Merge branch 'bug/JAL-2829deleteCharsWithGaps' into
authorgmungoc <g.m.carstairs@dundee.ac.uk>
Mon, 13 Nov 2017 11:59:46 +0000 (11:59 +0000)
committergmungoc <g.m.carstairs@dundee.ac.uk>
Mon, 13 Nov 2017 11:59:46 +0000 (11:59 +0000)
bug/JAL-2541cutRelocateFeatures

Conflicts:
src/jalview/datamodel/Sequence.java

13 files changed:
help/html/releases.html
src/jalview/commands/EditCommand.java
src/jalview/datamodel/Sequence.java
src/jalview/datamodel/SequenceI.java
src/jalview/gui/AlignViewport.java
src/jalview/gui/OverviewCanvas.java
src/jalview/gui/OverviewPanel.java
src/jalview/gui/StructureChooser.java
src/jalview/gui/StructureViewer.java
test/jalview/commands/EditCommandTest.java
test/jalview/datamodel/SequenceTest.java
test/jalview/gui/AlignViewportTest.java
test/jalview/gui/StructureViewerTest.java

index 7be088e..510e477 100755 (executable)
@@ -71,7 +71,7 @@ li:before {
       <td width="60" nowrap>
         <div align="center">
           <strong><a name="Jalview.2.10.3">2.10.3</a><br />
-            <em>10/10/2017</em></strong>
+            <em>14/11/2017</em></strong>
         </div>
       </td>
       <td><div align="left">
@@ -133,6 +133,8 @@ li:before {
             <li><!-- JAL-2973 -->Alignment ruler height set incorrectly after canceling the Alignment Window's Font dialog</li>
             <li><!-- JAL-2036 -->Show cross-references not enabled after restoring project until a new view is created</li>
             <li><!-- JAL-2756 -->Warning popup about use of SEQUENCE_ID in URL links appears when only default EMBL-EBI link is configured (since 2.10.2b2)</li>            
+            <li><!-- JAL-2775 -->Overview redraws whole window when box position is adjusted</li>
+            <li><!-- JAL-2225 -->Structure viewer doesn't map all chains in a multi-chain structure when viewing alignment involving more than one chain (since 2.10)</li>            
            </ul>
           <strong><em>Applet</em></strong><br/>
            <ul>
index d603a0d..f71fb79 100644 (file)
@@ -1415,43 +1415,45 @@ public class EditCommand implements CommandI
 
   public class Edit
   {
-    public SequenceI[] oldds;
+    private SequenceI[] oldds;
 
     /**
      * start and end of sequence prior to edit
      */
-    public Range[] oldStartEnd;
+    private Range[] oldStartEnd;
 
-    boolean fullAlignmentHeight = false;
+    private boolean fullAlignmentHeight = false;
 
-    Map<SequenceI, AlignmentAnnotation[]> deletedAnnotationRows;
+    private Map<SequenceI, AlignmentAnnotation[]> deletedAnnotationRows;
 
-    Map<String, Annotation[]> deletedAnnotations;
+    private Map<String, Annotation[]> deletedAnnotations;
 
     /*
      * features deleted by the cut (re-add on Undo)
      * (including the original of any shortened features)
      */
-    Map<SequenceI, List<SequenceFeature>> deletedFeatures;
+    private Map<SequenceI, List<SequenceFeature>> deletedFeatures;
 
     /*
      * shortened features added by the cut (delete on Undo)
      */
-    Map<SequenceI, List<SequenceFeature>> truncatedFeatures;
+    private Map<SequenceI, List<SequenceFeature>> truncatedFeatures;
 
-    AlignmentI al;
+    private AlignmentI al;
 
-    Action command;
+    final private Action command;
 
     char[][] string;
 
     SequenceI[] seqs;
 
-    int[] alIndex;
+    private int[] alIndex;
 
-    int position, number;
+    private int position;
 
-    char gapChar;
+    private int number;
+
+    private char gapChar;
 
     public Edit(Action cmd, SequenceI[] sqs, int pos, int count,
             char gap)
@@ -1479,6 +1481,16 @@ public class EditCommand implements CommandI
       fullAlignmentHeight = (align.getHeight() == sqs.length);
     }
 
+    /**
+     * Constructor given a REPLACE command and the replacement string
+     * 
+     * @param cmd
+     * @param sqs
+     * @param pos
+     * @param count
+     * @param align
+     * @param replace
+     */
     Edit(Action cmd, SequenceI[] sqs, int pos, int count,
             AlignmentI align, String replace)
     {
index 7efac54..006afb3 100755 (executable)
@@ -1162,7 +1162,7 @@ public class Sequence extends ASequence implements SequenceI
   }
 
   @Override
-  public void deleteChars(int i, int j)
+  public void deleteChars(final int i, final int j)
   {
     int newstart = start, newend = end;
     if (i >= sequence.length || i < 0)
@@ -1174,62 +1174,76 @@ public class Sequence extends ASequence implements SequenceI
     boolean createNewDs = false;
     // TODO: take a (second look) at the dataset creation validation method for
     // the very large sequence case
-    int eindex = -1, sindex = -1;
-    boolean ecalc = false, scalc = false;
+
+    int startIndex = findIndex(start) - 1;
+    int endIndex = findIndex(end) - 1;
+    int startDeleteColumn = -1; // for dataset sequence deletions
+    int deleteCount = 0;
+
     for (int s = i; s < j && s < sequence.length; s++)
     {
-      if (!Comparison.isGap(sequence[s]))
+      if (Comparison.isGap(sequence[s]))
+      {
+        continue;
+      }
+      deleteCount++;
+      if (startDeleteColumn == -1)
       {
-        if (createNewDs)
+        startDeleteColumn = findPosition(s) - start;
+      }
+      if (createNewDs)
+      {
+        newend--;
+      }
+      else
+      {
+        if (startIndex == s)
         {
-          newend--;
+          /*
+           * deleting characters from start of sequence; new start is the
+           * sequence position of the next column (position to the right
+           * if the column position is gapped)
+           */
+          newstart = findPosition(j);
+          break;
         }
         else
         {
-          if (!scalc)
-          {
-            sindex = findIndex(start) - 1;
-            scalc = true;
-          }
-          if (sindex == s)
+          if (endIndex < j)
           {
-            // delete characters including start of sequence
-            newstart = findPosition(j);
-            break; // don't need to search for any more residue characters.
+            /*
+             * deleting characters at end of sequence; new end is the sequence
+             * position of the column before the deletion; subtract 1 if this is
+             * gapped since findPosition returns the next sequence position
+             */
+            newend = findPosition(i - 1);
+            if (Comparison.isGap(sequence[i - 1]))
+            {
+              newend--;
+            }
+            break;
           }
           else
           {
-            // delete characters after start.
-            if (!ecalc)
-            {
-              eindex = findIndex(end) - 1;
-              ecalc = true;
-            }
-            if (eindex < j)
-            {
-              // delete characters at end of sequence
-              newend = findPosition(i - 1);
-              break; // don't need to search for any more residue characters.
-            }
-            else
-            {
-              createNewDs = true;
-              newend--; // decrease end position by one for the deleted residue
-              // and search further
-            }
+            createNewDs = true;
+            newend--;
           }
         }
       }
     }
-    // deletion occured in the middle of the sequence
+
     if (createNewDs && this.datasetSequence != null)
     {
-      // construct a new sequence
+      /*
+       * if deletion occured in the middle of the sequence,
+       * construct a new dataset sequence and delete the residues
+       * that were deleted from the aligned sequence
+       */
       Sequence ds = new Sequence(datasetSequence);
+      ds.deleteChars(startDeleteColumn, startDeleteColumn + deleteCount);
+      datasetSequence = ds;
       // TODO: remove any non-inheritable properties ?
       // TODO: create a sequence mapping (since there is a relation here ?)
-      ds.deleteChars(i, j);
-      datasetSequence = ds;
     }
     start = newstart;
     end = newend;
@@ -1680,7 +1694,7 @@ public class Sequence extends ASequence implements SequenceI
   }
 
   /**
-   * @return The index (zero-based) on this sequence in the MSA. It returns
+   * @return The index (zero-based) of this sequence in the MSA. It returns
    *         {@code -1} if this information is not available.
    */
   @Override
@@ -1690,12 +1704,10 @@ public class Sequence extends ASequence implements SequenceI
   }
 
   /**
-   * Defines the position of this sequence in the MSA. Use the value {@code -1}
-   * if this information is undefined.
+   * Defines the (zero-based) position of this sequence in the MSA. Use the
+   * value {@code -1} if this information is undefined.
    * 
-   * @param The
-   *          position for this sequence. This value is zero-based (zero for
-   *          this first sequence)
+   * @param value
    */
   @Override
   public void setIndex(int value)
index 014a6e5..f1a4140 100755 (executable)
@@ -192,13 +192,14 @@ public interface SequenceI extends ASequenceI
   public int findIndex(int pos);
 
   /**
-   * Returns the sequence position for an alignment position.
+   * Returns the sequence position for an alignment (column) position. If at a
+   * gap, returns the position of the next residue to the right. If beyond the
+   * end of the sequence, returns 1 more than the last residue position.
    * 
    * @param i
    *          column index in alignment (from 0..<length)
    * 
-   * @return TODO: JAL-2562 - residue number for residue (left of and) nearest
-   *         ith column
+   * @return
    */
   public int findPosition(int i);
 
index 90271c8..4d09084 100644 (file)
@@ -583,58 +583,6 @@ public class AlignViewport extends AlignmentViewport
             .getStructureSelectionManager(Desktop.instance);
   }
 
-  /**
-   * 
-   * @param pdbEntries
-   * @return an array of SequenceI arrays, one for each PDBEntry, listing which
-   *         sequences in the alignment hold a reference to it
-   */
-  public SequenceI[][] collateForPDB(PDBEntry[] pdbEntries)
-  {
-    List<SequenceI[]> seqvectors = new ArrayList<SequenceI[]>();
-    for (PDBEntry pdb : pdbEntries)
-    {
-      List<SequenceI> choosenSeqs = new ArrayList<SequenceI>();
-      for (SequenceI sq : alignment.getSequences())
-      {
-        Vector<PDBEntry> pdbRefEntries = sq.getDatasetSequence()
-                .getAllPDBEntries();
-        if (pdbRefEntries == null)
-        {
-          continue;
-        }
-        for (PDBEntry pdbRefEntry : pdbRefEntries)
-        {
-          if (pdbRefEntry.getId().equals(pdb.getId()))
-          {
-            if (pdbRefEntry.getChainCode() != null
-                    && pdb.getChainCode() != null)
-            {
-              if (pdbRefEntry.getChainCode().equalsIgnoreCase(
-                      pdb.getChainCode()) && !choosenSeqs.contains(sq))
-              {
-                choosenSeqs.add(sq);
-                continue;
-              }
-            }
-            else
-            {
-              if (!choosenSeqs.contains(sq))
-              {
-                choosenSeqs.add(sq);
-                continue;
-              }
-            }
-
-          }
-        }
-      }
-      seqvectors
-              .add(choosenSeqs.toArray(new SequenceI[choosenSeqs.size()]));
-    }
-    return seqvectors.toArray(new SequenceI[seqvectors.size()][]);
-  }
-
   @Override
   public boolean isNormaliseSequenceLogo()
   {
index 7371eb5..2991889 100644 (file)
@@ -183,6 +183,8 @@ public class OverviewCanvas extends JComponent
   @Override
   public void paintComponent(Graphics g)
   {
+    // super.paintComponent(g);
+
     if (restart)
     {
       if (lastMiniMe == null)
@@ -204,7 +206,8 @@ public class OverviewCanvas extends JComponent
               && ((getWidth() != od.getWidth())
                       || (getHeight() != od.getHeight())))
       {
-        // if there is annotation, scale the alignment and annotation separately
+        // if there is annotation, scale the alignment and annotation
+        // separately
         if (od.getGraphHeight() > 0)
         {
           BufferedImage topImage = lastMiniMe.getSubimage(0, 0,
@@ -235,25 +238,24 @@ public class OverviewCanvas extends JComponent
           od.setHeight(getHeight());
         }
 
-        // scale lastMiniMe to the new size
-        g.drawImage(lastMiniMe, 0, 0, getWidth(), getHeight(), this);
-
         // make sure the box is in the right place
         od.setBoxPosition(av.getAlignment().getHiddenSequences(),
                 av.getAlignment().getHiddenColumns());
       }
-      else // not a resize
-      {
-        // fall back to normal behaviour
-        g.drawImage(lastMiniMe, 0, 0, getWidth(), getHeight(), this);
-      }
+      // fall back to normal behaviour
+      g.drawImage(lastMiniMe, 0, 0, getWidth(), getHeight(), this);
     }
-
+    else
+    {
+      g.drawImage(lastMiniMe, 0, 0, getWidth(), getHeight(), this);
+    }
+    
     // draw the box
     g.setColor(Color.red);
     od.drawBox(g);
   }
 
+
   public void dispose()
   {
     dispose = true;
index 9ddb751..43b4310 100755 (executable)
@@ -329,6 +329,22 @@ public class OverviewPanel extends JPanel
    * changed
    * 
    */
+  private void setBoxPositionOnly()
+  {
+    if (od != null)
+    {
+      int oldX = od.getBoxX();
+      int oldY = od.getBoxY();
+      int oldWidth = od.getBoxWidth();
+      int oldHeight = od.getBoxHeight();
+      od.setBoxPosition(av.getAlignment().getHiddenSequences(),
+              av.getAlignment().getHiddenColumns());
+      repaint(oldX - 1, oldY - 1, oldWidth + 2, oldHeight + 2);
+      repaint(od.getBoxX(), od.getBoxY(), od.getBoxWidth(),
+              od.getBoxHeight());
+    }
+  }
+
   private void setBoxPosition()
   {
     if (od != null)
@@ -342,7 +358,7 @@ public class OverviewPanel extends JPanel
   @Override
   public void propertyChange(PropertyChangeEvent evt)
   {
-    setBoxPosition();
+    setBoxPositionOnly();
   }
 
   /**
index 20f4a49..37632ef 100644 (file)
@@ -927,16 +927,10 @@ public class StructureChooser extends GStructureChooser
     }
     if (pdbEntriesToView.length > 1)
     {
-      ArrayList<SequenceI[]> seqsMap = new ArrayList<>();
-      for (SequenceI seq : sequences)
-      {
-        seqsMap.add(new SequenceI[] { seq });
-      }
-      SequenceI[][] collatedSeqs = seqsMap.toArray(new SequenceI[0][0]);
-
-      setProgressBar(MessageManager
-                    .getString("status.fetching_3d_structures_for_selected_entries"), progressId);
-      sViewer.viewStructures(pdbEntriesToView, collatedSeqs, alignPanel);
+      setProgressBar(MessageManager.getString(
+              "status.fetching_3d_structures_for_selected_entries"),
+              progressId);
+      sViewer.viewStructures(pdbEntriesToView, sequences, alignPanel);
     }
     else
     {
index e58b378..b142613 100644 (file)
@@ -29,19 +29,24 @@ 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.
- * 
- * this allows new views to be created with the currently configured viewer, the
- * preferred viewer to be set/read and existing views created previously with a
- * particular viewer to be recovered
+ * A proxy for handling structure viewers, that orchestrates adding selected
+ * structures, associated with sequences in Jalview, to an existing viewer, or
+ * opening a new one. Currently supports either Jmol or Chimera as the structure
+ * viewer.
  * 
  * @author jprocter
  */
 public class StructureViewer
 {
+  private static final String UNKNOWN_VIEWER_TYPE = "Unknown structure viewer type ";
+
   StructureSelectionManager ssm;
 
   public enum ViewerType
@@ -49,6 +54,16 @@ public class StructureViewer
     JMOL, CHIMERA
   };
 
+  /**
+   * Constructor
+   * 
+   * @param structureSelectionManager
+   */
+  public StructureViewer(StructureSelectionManager structureSelectionManager)
+  {
+    ssm = structureSelectionManager;
+  }
+
   public ViewerType getViewerType()
   {
     String viewType = Cache.getDefault(Preferences.STRUCTURE_DISPLAY,
@@ -61,135 +76,157 @@ public class StructureViewer
     Cache.setProperty(Preferences.STRUCTURE_DISPLAY, type.name());
   }
 
-  public StructureViewer(
-          StructureSelectionManager structureSelectionManager)
-  {
-    ssm = structureSelectionManager;
-  }
-
   /**
    * View multiple PDB entries, each with associated sequences
    * 
    * @param pdbs
-   * @param seqsForPdbs
+   * @param seqs
    * @param ap
    * @return
    */
   public JalviewStructureDisplayI viewStructures(PDBEntry[] pdbs,
-          SequenceI[][] seqsForPdbs, AlignmentPanel ap)
+          SequenceI[] seqs, AlignmentPanel ap)
   {
-    JalviewStructureDisplayI viewer = onlyOnePdb(pdbs, seqsForPdbs, ap);
+    JalviewStructureDisplayI viewer = onlyOnePdb(pdbs, seqs, ap);
     if (viewer != null)
     {
+      /*
+       * user added structure to an existing viewer - all done
+       */
       return viewer;
     }
-    return viewStructures(getViewerType(), pdbs, seqsForPdbs, ap);
+
+    ViewerType viewerType = getViewerType();
+
+    Map<PDBEntry, SequenceI[]> seqsForPdbs = getSequencesForPdbs(pdbs,
+            seqs);
+    PDBEntry[] pdbsForFile = seqsForPdbs.keySet().toArray(
+            new PDBEntry[seqsForPdbs.size()]);
+    SequenceI[][] theSeqs = seqsForPdbs.values().toArray(
+            new SequenceI[seqsForPdbs.size()][]);
+    JalviewStructureDisplayI sview = null;
+    if (viewerType.equals(ViewerType.JMOL))
+    {
+      sview = new AppJmol(ap, pdbsForFile, theSeqs);
+    }
+    else if (viewerType.equals(ViewerType.CHIMERA))
+    {
+      sview = new ChimeraViewFrame(pdbsForFile, theSeqs, ap);
+    }
+    else
+    {
+      Cache.log.error(UNKNOWN_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)
+  Map<PDBEntry, SequenceI[]> getSequencesForPdbs(PDBEntry[] pdbs,
+          SequenceI[] seqs)
   {
-    List<SequenceI> seqs = new ArrayList<SequenceI>();
-    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<String, PDBEntry> pdbsSeen = new HashMap<>();
+
+    /*
+     * LinkedHashMap preserves order of PDB entries (significant if they
+     * will get superimposed to the first structure)
+     */
+    Map<PDBEntry, List<SequenceI>> 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>());
+      }
+      else
+      {
+        pdb = pdbsSeen.get(pdbFile);
       }
-      SequenceI[] pdbseqs = seqsForPdbs[i++];
-      if (pdbseqs != null)
+      List<SequenceI> seqsForPdb = pdbSeqs.get(pdb);
+      if (!seqsForPdb.contains(seq))
       {
-        for (SequenceI sq : pdbseqs)
-        {
-          seqs.add(sq);
-        }
+        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))
-    {
-      sview = new AppJmol(ap, pdbsForFile,
-              ap.av.collateForPDB(pdbsForFile));
-    }
-    else if (viewerType.equals(ViewerType.CHIMERA))
+    /*
+     * convert to Map<PDBEntry, SequenceI[]>
+     */
+    Map<PDBEntry, SequenceI[]> result = new LinkedHashMap<>();
+    for (Entry<PDBEntry, List<SequenceI>> entry : pdbSeqs.entrySet())
     {
-      sview = new ChimeraViewFrame(pdbsForFile,
-              ap.av.collateForPDB(pdbsForFile), ap);
+      List<SequenceI> theSeqs = entry.getValue();
+      result.put(entry.getKey(),
+              theSeqs.toArray(new SequenceI[theSeqs.size()]));
     }
-    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<SequenceI> seqs = new ArrayList<SequenceI>();
+    if (pdbs == null || pdbs.length == 0)
     {
       return null;
     }
-    List<PDBEntry> uniques = new ArrayList<PDBEntry>();
-    List<String> filesSeen = new ArrayList<String>();
-    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))
     {
@@ -201,8 +238,7 @@ public class StructureViewer
     }
     else
     {
-      Cache.log.error("Unknown structure viewer type "
-              + getViewerType().toString());
+      Cache.log.error(UNKNOWN_VIEWER_TYPE + getViewerType().toString());
     }
     return sview;
   }
@@ -242,7 +278,7 @@ public class StructureViewer
               "Unsupported structure viewer type " + type.toString());
       break;
     default:
-      Cache.log.error("Unknown structure viewer type " + type.toString());
+      Cache.log.error(UNKNOWN_VIEWER_TYPE + type.toString());
     }
     return sview;
   }
index 9547d8e..a0c2ccf 100644 (file)
@@ -155,7 +155,7 @@ public class EditCommandTest
   /**
    * Test a Paste action, where this adds sequences to an alignment.
    */
-  @Test(groups = { "Functional" }, enabled = false)
+  @Test(groups = { "Functional" }, enabled = true)
   // TODO fix so it works
   public void testPaste_addToAlignment()
   {
@@ -870,6 +870,7 @@ public class EditCommandTest
           assertEquals("Wrong Dataset size after cut", 1,
                   alignment.getDataset().getHeight());
         }
+
         /*
          * redo and verify
          */
@@ -892,6 +893,7 @@ public class EditCommandTest
                   newDatasetSequence ? 2 : 1, alignment.getDataset()
                           .getHeight());
         }
+
         /*
          * undo ready for next cut
          */
index 819a8f8..833d957 100644 (file)
@@ -1828,4 +1828,82 @@ public class SequenceTest
     sq.checkValidRange();
     assertEquals(22, sq.getEnd());
   }
+
+  @Test(groups = { "Functional" })
+  public void testDeleteChars_withGaps()
+  {
+    /*
+     * delete gaps only
+     */
+    SequenceI sq = new Sequence("test/8-10", "A-B-C");
+    sq.createDatasetSequence();
+    assertEquals("ABC", sq.getDatasetSequence().getSequenceAsString());
+    sq.deleteChars(1, 2); // delete first gap
+    assertEquals("AB-C", sq.getSequenceAsString());
+    assertEquals(8, sq.getStart());
+    assertEquals(10, sq.getEnd());
+    assertEquals("ABC", sq.getDatasetSequence().getSequenceAsString());
+
+    /*
+     * delete gaps and residues at start (no new dataset sequence)
+     */
+    sq = new Sequence("test/8-10", "A-B-C");
+    sq.createDatasetSequence();
+    sq.deleteChars(0, 3); // delete A-B
+    assertEquals("-C", sq.getSequenceAsString());
+    assertEquals(10, sq.getStart());
+    assertEquals(10, sq.getEnd());
+    assertEquals("ABC", sq.getDatasetSequence().getSequenceAsString());
+
+    /*
+     * delete gaps and residues at end (no new dataset sequence)
+     */
+    sq = new Sequence("test/8-10", "A-B-C");
+    sq.createDatasetSequence();
+    sq.deleteChars(2, 5); // delete B-C
+    assertEquals("A-", sq.getSequenceAsString());
+    assertEquals(8, sq.getStart());
+    assertEquals(8, sq.getEnd());
+    assertEquals("ABC", sq.getDatasetSequence().getSequenceAsString());
+
+    /*
+     * delete gaps and residues internally (new dataset sequence)
+     * first delete from gap to residue
+     */
+    sq = new Sequence("test/8-10", "A-B-C");
+    sq.createDatasetSequence();
+    sq.deleteChars(1, 3); // delete -B
+    assertEquals("A-C", sq.getSequenceAsString());
+    assertEquals(8, sq.getStart());
+    assertEquals(9, sq.getEnd());
+    assertEquals("AC", sq.getDatasetSequence().getSequenceAsString());
+    assertEquals(8, sq.getDatasetSequence().getStart());
+    assertEquals(9, sq.getDatasetSequence().getEnd());
+
+    /*
+     * internal delete from gap to gap
+     */
+    sq = new Sequence("test/8-10", "A-B-C");
+    sq.createDatasetSequence();
+    sq.deleteChars(1, 4); // delete -B-
+    assertEquals("AC", sq.getSequenceAsString());
+    assertEquals(8, sq.getStart());
+    assertEquals(9, sq.getEnd());
+    assertEquals("AC", sq.getDatasetSequence().getSequenceAsString());
+    assertEquals(8, sq.getDatasetSequence().getStart());
+    assertEquals(9, sq.getDatasetSequence().getEnd());
+
+    /*
+     * internal delete from residue to residue
+     */
+    sq = new Sequence("test/8-10", "A-B-C");
+    sq.createDatasetSequence();
+    sq.deleteChars(2, 3); // delete B
+    assertEquals("A--C", sq.getSequenceAsString());
+    assertEquals(8, sq.getStart());
+    assertEquals(9, sq.getEnd());
+    assertEquals("AC", sq.getDatasetSequence().getSequenceAsString());
+    assertEquals(8, sq.getDatasetSequence().getStart());
+    assertEquals(9, sq.getDatasetSequence().getEnd());
+  }
 }
index 812fd8f..5ed0cac 100644 (file)
@@ -96,57 +96,6 @@ public class AlignViewportTest
     testee = new AlignViewport(al);
   }
 
-  @Test(groups = { "Functional" })
-  public void testCollateForPdb()
-  {
-    // JBP: What behaviour is this supposed to test ?
-    /*
-     * Set up sequence pdb ids
-     */
-    PDBEntry pdb1 = new PDBEntry("1ABC", "B", Type.PDB, "1ABC.pdb");
-    PDBEntry pdb2 = new PDBEntry("2ABC", "C", Type.PDB, "2ABC.pdb");
-    PDBEntry pdb3 = new PDBEntry("3ABC", "D", Type.PDB, "3ABC.pdb");
-
-    /*
-     * seq1 and seq3 refer to 1abcB, seq2 to 2abcC, none to 3abcD
-     */
-    al.getSequenceAt(0).getDatasetSequence()
-            .addPDBId(new PDBEntry("1ABC", "B", Type.PDB, "1ABC.pdb"));
-    al.getSequenceAt(2).getDatasetSequence()
-            .addPDBId(new PDBEntry("1ABC", "B", Type.PDB, "1ABC.pdb"));
-    al.getSequenceAt(1).getDatasetSequence()
-            .addPDBId(new PDBEntry("2ABC", "C", Type.PDB, "2ABC.pdb"));
-    /*
-     * Add a second chain PDB xref to Seq2 - should not result in a duplicate in
-     * the results
-     */
-    al.getSequenceAt(1).getDatasetSequence()
-            .addPDBId(new PDBEntry("2ABC", "D", Type.PDB, "2ABC.pdb"));
-    /*
-     * Seq3 refers to 3abc - this does not match 3ABC (as the code stands)
-     */
-    al.getSequenceAt(2).getDatasetSequence()
-            .addPDBId(new PDBEntry("3abc", "D", Type.PDB, "3ABC.pdb"));
-
-    /*
-     * run method under test
-     */
-    SequenceI[][] seqs = testee.collateForPDB(new PDBEntry[] { pdb1, pdb2,
-        pdb3 });
-
-    // seq1 and seq3 refer to PDBEntry[0]
-    assertEquals(2, seqs[0].length);
-    assertSame(al.getSequenceAt(0), seqs[0][0]);
-    assertSame(al.getSequenceAt(2), seqs[0][1]);
-
-    // seq2 refers to PDBEntry[1]
-    assertEquals(1, seqs[1].length);
-    assertSame(al.getSequenceAt(1), seqs[1][0]);
-
-    // no sequence refers to PDBEntry[2]
-    assertEquals(0, seqs[2].length);
-  }
-
   /**
    * Test that a mapping is not deregistered when a second view is closed but
    * the first still holds a reference to the mapping
index c1c1d5c..4d5b114 100644 (file)
@@ -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,11 @@ public class StructureViewerTest
   }
 
   @Test(groups = "Functional")
-  public void testGetUniquePdbFiles()
+  public void testGetSequencesForPdbs()
   {
-    assertNull(StructureViewer.getUniquePdbFiles(null));
+    StructureViewer sv = new StructureViewer(null);
+
+    assertNull(sv.getSequencesForPdbs(null, null));
 
     PDBEntry pdbe1 = new PDBEntry("1A70", "A", Type.PDB, "path1");
     PDBEntry pdbe2 = new PDBEntry("3A6S", "A", Type.PDB, "path2");
@@ -30,13 +39,45 @@ 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<PDBEntry, SequenceI[]> uniques = sv.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]);
   }
 }