JAL-653 mappings always reside on the dataset alignment if there is one

diff --git a/src/jalview/datamodel/Alignment.java b/src/jalview/datamodel/Alignment.java
index 7ea9985..3f9f03f 100755 (executable)
--- a/src/jalview/datamodel/Alignment.java
+++ b/src/jalview/datamodel/Alignment.java
@@ -108,7 +108,7 @@ public class Alignment implements AlignmentI
      * Share the same dataset sequence mappings (if any). TODO: find a better
      * place for these to live (alignment dataset?).
      */
-    this.codonFrameList = ((Alignment) al).codonFrameList;
+    this.setCodonFrames(al.getCodonFrames());
 
     initAlignment(seqs);
   }
@@ -991,25 +991,7 @@ public class Alignment implements AlignmentI
   {
     if (dataset == null && data == null)
     {
-      // Create a new dataset for this alignment.
-      // Can only be done once, if dataset is not null
-      // This will not be performed
-      SequenceI[] seqs = new SequenceI[getHeight()];
-      SequenceI currentSeq;
-      for (int i = 0; i < getHeight(); i++)
-      {
-        currentSeq = getSequenceAt(i);
-        if (currentSeq.getDatasetSequence() != null)
-        {
-          seqs[i] = currentSeq.getDatasetSequence();
-        }
-        else
-        {
-          seqs[i] = currentSeq.createDatasetSequence();
-        }
-      }
-
-      dataset = new Alignment(seqs);
+      createDatasetAlignment();
     }
     else if (dataset == null && data != null)
     {
@@ -1040,6 +1022,37 @@ public class Alignment implements AlignmentI
   }
 
   /**
+   * Creates a new dataset for this alignment. Can only be done once - if
+   * dataset is not null this will not be performed.
+   */
+  public void createDatasetAlignment()
+  {
+    if (dataset != null)
+    {
+      return;
+    }
+    SequenceI[] seqs = new SequenceI[getHeight()];
+    SequenceI currentSeq;
+    for (int i = 0; i < getHeight(); i++)
+    {
+      currentSeq = getSequenceAt(i);
+      if (currentSeq.getDatasetSequence() != null)
+      {
+        seqs[i] = currentSeq.getDatasetSequence();
+      }
+      else
+      {
+        seqs[i] = currentSeq.createDatasetSequence();
+      }
+    }
+
+    dataset = new Alignment(seqs);
+    // move mappings to the dataset alignment
+    dataset.codonFrameList = this.codonFrameList;
+    this.codonFrameList = null;
+  }
+
+  /**
    * reference count for number of alignments referencing this one.
    */
   int alignmentRefs = 0;
@@ -1261,19 +1274,17 @@ public class Alignment implements AlignmentI
     return alignmentProperties;
   }
 
-  /*
-   * (non-Javadoc)
-   * 
-   * @see
-   * jalview.datamodel.AlignmentI#addCodonFrame(jalview.datamodel.AlignedCodonFrame
-   * )
+  /**
+   * Adds the given mapping to the stored set. Note this may be held on the
+   * dataset alignment.
    */
   @Override
   public void addCodonFrame(AlignedCodonFrame codons)
   {
-    if (codons != null)
+    Set<AlignedCodonFrame> acfs = getCodonFrames();
+    if (codons != null && acfs != null)
     {
-      codonFrameList.add(codons);
+      acfs.add(codons);
     }
   }
 
@@ -1291,7 +1302,7 @@ public class Alignment implements AlignmentI
       return null;
     }
     List<AlignedCodonFrame> cframes = new ArrayList<AlignedCodonFrame>();
-    for (AlignedCodonFrame acf : codonFrameList)
+    for (AlignedCodonFrame acf : getCodonFrames())
     {
       if (acf.involvesSequence(seq))
       {
@@ -1302,42 +1313,50 @@ public class Alignment implements AlignmentI
   }
 
   /**
-   * Sets the codon frame mappings (replacing any existing mappings).
+   * Sets the codon frame mappings (replacing any existing mappings). Note the
+   * mappings are set on the dataset alignment instead if there is one.
    * 
    * @see jalview.datamodel.AlignmentI#setCodonFrames()
    */
   @Override
   public void setCodonFrames(Set<AlignedCodonFrame> acfs)
   {
-    this.codonFrameList = acfs;
+    if (dataset != null)
+    {
+      dataset.setCodonFrames(acfs);
+    }
+    else
+    {
+      this.codonFrameList = acfs;
+    }
   }
 
   /**
    * Returns the set of codon frame mappings. Any changes to the returned set
-   * will affect the alignment.
+   * will affect the alignment. The mappings are held on (and read from) the
+   * dataset alignment if there is one.
    * 
    * @see jalview.datamodel.AlignmentI#getCodonFrames()
    */
   @Override
   public Set<AlignedCodonFrame> getCodonFrames()
   {
-    return codonFrameList;
+    return dataset != null ? dataset.getCodonFrames() : codonFrameList;
   }
 
-  /*
-   * (non-Javadoc)
-   * 
-   * @seejalview.datamodel.AlignmentI#removeCodonFrame(jalview.datamodel.
-   * AlignedCodonFrame)
+  /**
+   * Removes the given mapping from the stored set. Note that the mappings are
+   * held on the dataset alignment if there is one.
    */
   @Override
   public boolean removeCodonFrame(AlignedCodonFrame codons)
   {
-    if (codons == null || codonFrameList == null)
+    Set<AlignedCodonFrame> acfs = getCodonFrames();
+    if (codons == null || acfs == null)
     {
       return false;
     }
-    return codonFrameList.remove(codons);
+    return acfs.remove(codons);
   }
 
   @Override
@@ -1383,7 +1402,7 @@ public class Alignment implements AlignmentI
       addAnnotation(alan[a]);
     }
 
-    this.codonFrameList.addAll(toappend.getCodonFrames());
+    getCodonFrames().addAll(toappend.getCodonFrames());
 
     List<SequenceGroup> sg = toappend.getGroups();
     if (sg != null)
@@ -1595,6 +1614,7 @@ public class Alignment implements AlignmentI
    * 
    * @return the representative sequence for this group
    */
+  @Override
   public SequenceI getSeqrep()
   {
     return seqrep;
@@ -1607,6 +1627,7 @@ public class Alignment implements AlignmentI
    * @param seqrep
    *          the seqrep to set (null means no sequence representative)
    */
+  @Override
   public void setSeqrep(SequenceI seqrep)
   {
     this.seqrep = seqrep;
@@ -1616,6 +1637,7 @@ public class Alignment implements AlignmentI
    * 
    * @return true if group has a sequence representative
    */
+  @Override
   public boolean hasSeqrep()
   {
     return seqrep != null;

diff --git a/test/jalview/datamodel/AlignmentTest.java b/test/jalview/datamodel/AlignmentTest.java
index 5d299bb..ad1d3bf 100644 (file)
--- a/test/jalview/datamodel/AlignmentTest.java
+++ b/test/jalview/datamodel/AlignmentTest.java
@@ -22,6 +22,8 @@ package jalview.datamodel;
 
 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 static org.testng.AssertJUnit.assertSame;
 import static org.testng.AssertJUnit.assertTrue;
 
@@ -392,6 +394,7 @@ public class AlignmentTest
   public void testCopyConstructor() throws IOException
   {
     AlignmentI protein = loadAlignment(AA_SEQS_1, FormatAdapter.PASTE);
+    // create sequence and alignment datasets
     protein.setDataset(null);
     AlignedCodonFrame acf = new AlignedCodonFrame();
     protein.getDataset().setCodonFrames(Collections.singleton(acf));
@@ -406,8 +409,63 @@ public class AlignmentTest
             .getSequenceAt(0).getDatasetSequence());
     assertSame(copy.getSequenceAt(1).getDatasetSequence(), protein
             .getSequenceAt(1).getDatasetSequence());
-    // and the same alignment dataset (?)
-    // or a new one with the same dataset sequences?
-    assertSame(copy.getDataset(), protein.getDataset());
+
+    // TODO should the copy constructor copy the dataset?
+    // or make a new one referring to the same dataset sequences??
+    assertNull(copy.getDataset());
+    // assertArrayEquals(copy.getDataset().getSequencesArray(), protein
+    // .getDataset().getSequencesArray());
+  }
+
+  /**
+   * Test behaviour of createDataset
+   * 
+   * @throws IOException
+   */
+  @Test(groups = "Functional")
+  public void testCreateDatasetAlignment() throws IOException
+  {
+    AlignmentI protein = new FormatAdapter().readFile(AA_SEQS_1,
+            AppletFormatAdapter.PASTE, "FASTA");
+    /*
+     * create a dataset sequence on first sequence
+     * leave the second without one
+     */
+    protein.getSequenceAt(0).createDatasetSequence();
+    assertNotNull(protein.getSequenceAt(0).getDatasetSequence());
+    assertNull(protein.getSequenceAt(1).getDatasetSequence());
+
+    /*
+     * add a mapping to the alignment
+     */
+    AlignedCodonFrame acf = new AlignedCodonFrame();
+    protein.addCodonFrame(acf);
+    assertNull(protein.getDataset());
+    assertTrue(protein.getCodonFrames().contains(acf));
+
+    /*
+     * create the alignment dataset
+     * note this creates sequence datasets where missing
+     * as a side-effect (in this case, on seq2
+     */
+    // TODO promote this method to AlignmentI
+    ((Alignment) protein).createDatasetAlignment();
+
+    // TODO this method should return AlignmentI not Alignment !!
+    Alignment ds = protein.getDataset();
+
+    // side-effect: dataset created on second sequence
+    assertNotNull(protein.getSequenceAt(1).getDatasetSequence());
+    // dataset alignment has references to dataset sequences
+    assertEquals(ds.getSequenceAt(0), protein.getSequenceAt(0)
+            .getDatasetSequence());
+    assertEquals(ds.getSequenceAt(1), protein.getSequenceAt(1)
+            .getDatasetSequence());
+
+    // codon frames should have been moved to the dataset
+    // getCodonFrames() should delegate to the dataset:
+    assertTrue(protein.getCodonFrames().contains(acf));
+    // prove the codon frames are indeed on the dataset:
+    assertTrue(ds.getCodonFrames().contains(acf));
   }
 }