Merge branch 'bug/JAL-2541cutWithFeatures' into features/JAL-2446NCList
authorgmungoc <g.m.carstairs@dundee.ac.uk>
Wed, 24 May 2017 14:21:43 +0000 (15:21 +0100)
committergmungoc <g.m.carstairs@dundee.ac.uk>
Wed, 24 May 2017 14:21:43 +0000 (15:21 +0100)
1  2 
src/jalview/commands/EditCommand.java
src/jalview/datamodel/Sequence.java
test/jalview/datamodel/SequenceTest.java

@@@ -555,6 -555,6 +555,7 @@@ public class EditCommand implements Com
                command.oldds = new SequenceI[command.seqs.length];
              }
              command.oldds[i] = oldds;
++            // FIXME JAL-2541 JAL-2526 get correct positions if on a gap
              adjustFeatures(
                      command,
                      i,
      }
    }
  
--  final static void adjustFeatures(Edit command, int index, int i, int j,
--          boolean insert)
++  final static void adjustFeatures(Edit command, int index, final int i,
++          final int j, boolean insert)
    {
      SequenceI seq = command.seqs[index];
      SequenceI sequence = seq.getDatasetSequence();
        return;
      }
  
 -    SequenceFeature[] sf = sequence.getSequenceFeatures();
 +    List<SequenceFeature> sf = sequence.getFeatures()
 +            .getPositionalFeatures();
  
 -    if (sf == null)
 +    if (sf.isEmpty())
      {
        return;
      }
  
 -    SequenceFeature[] oldsf = new SequenceFeature[sf.length];
 +    SequenceFeature[] oldsf = new SequenceFeature[sf.size()];
  
      int cSize = j - i;
  
 -    for (int s = 0; s < sf.length; s++)
 +    int s = 0;
 +    for (SequenceFeature feature : sf)
      {
 -      SequenceFeature copy = new SequenceFeature(sf[s]);
 +      SequenceFeature copy = new SequenceFeature(feature);
  
 -      oldsf[s] = copy;
 +      oldsf[s++] = copy;
  
 -      if (sf[s].getEnd() < i)
 +      if (feature.getEnd() < i)
        {
          continue;
        }
  
 -      if (sf[s].getBegin() > j)
 +      if (feature.getBegin() > j)
        {
 -        sf[s].setBegin(copy.getBegin() - cSize);
 -        sf[s].setEnd(copy.getEnd() - cSize);
 +        int newBegin = copy.getBegin() - cSize;
 +        int newEnd = copy.getEnd() - cSize;
 +        SequenceFeature newSf = new SequenceFeature(feature, newBegin,
 +                newEnd, feature.getFeatureGroup());
 +        sequence.deleteFeature(feature);
 +        sequence.addSequenceFeature(newSf);
 +        // feature.setBegin(newBegin);
 +        // feature.setEnd(newEnd);
          continue;
        }
  
 -      if (sf[s].getBegin() >= i)
 +      int newBegin = feature.getBegin();
 +      int newEnd = feature.getEnd();
 +      if (newBegin >= i)
        {
 -        sf[s].setBegin(i);
 +        newBegin = i;
 +        // feature.setBegin(i);
        }
  
 -      if (sf[s].getEnd() < j)
 +      if (newEnd < j)
        {
 -        sf[s].setEnd(j - 1);
 +        newEnd = j - 1;
 +        // feature.setEnd(j - 1);
        }
 +      newEnd = newEnd - cSize;
 +      // feature.setEnd(feature.getEnd() - (cSize));
  
 -      sf[s].setEnd(sf[s].getEnd() - (cSize));
 -
 -      if (sf[s].getBegin() > sf[s].getEnd())
 +      sequence.deleteFeature(feature);
 +      if (newEnd >= newBegin)
        {
 -        sequence.deleteFeature(sf[s]);
 +        sequence.addSequenceFeature(new SequenceFeature(feature, newBegin,
 +                newEnd, feature.getFeatureGroup()));
        }
 +      // if (feature.getBegin() > feature.getEnd())
 +      // {
 +      // sequence.deleteFeature(feature);
 +      // }
      }
  
      if (command.editedFeatures == null)
@@@ -22,8 -22,6 +22,8 @@@ package jalview.datamodel
  
  import jalview.analysis.AlignSeq;
  import jalview.api.DBRefEntryI;
 +import jalview.datamodel.features.SequenceFeatures;
 +import jalview.datamodel.features.SequenceFeaturesI;
  import jalview.util.Comparison;
  import jalview.util.DBRefUtils;
  import jalview.util.MapList;
@@@ -36,6 -34,8 +36,8 @@@ import java.util.Enumeration
  import java.util.List;
  import java.util.Vector;
  
+ import com.stevesoft.pat.Regex;
  import fr.orsay.lri.varna.models.rna.RNA;
  
  /**
   */
  public class Sequence extends ASequence implements SequenceI
  {
+   private static final Regex limitrx = new Regex(
+           "[/][0-9]{1,}[-][0-9]{1,}$");
+   private static final Regex endrx = new Regex("[0-9]{1,}$");
    SequenceI datasetSequence;
  
    String name;
@@@ -83,8 -88,6 +90,8 @@@
    /** array of sequence features - may not be null for a valid sequence object */
    public SequenceFeature[] sequenceFeatures;
  
 +  private SequenceFeatures sequenceFeatureStore;
 +
    /**
     * Creates a new Sequence object.
     * 
      this.sequence = sequence2;
      this.start = start2;
      this.end = end2;
 +    sequenceFeatureStore = new SequenceFeatures();
      parseId();
      checkValidRange();
    }
  
-   com.stevesoft.pat.Regex limitrx = new com.stevesoft.pat.Regex(
-           "[/][0-9]{1,}[-][0-9]{1,}$");
-   com.stevesoft.pat.Regex endrx = new com.stevesoft.pat.Regex("[0-9]{1,}$");
    void parseId()
    {
      if (name == null)
    protected void initSeqFrom(SequenceI seq,
            AlignmentAnnotation[] alAnnotation)
    {
-     {
-       char[] oseq = seq.getSequence();
-       initSeqAndName(seq.getName(), Arrays.copyOf(oseq, oseq.length),
-               seq.getStart(), seq.getEnd());
-     }
+     char[] oseq = seq.getSequence();
+     initSeqAndName(seq.getName(), Arrays.copyOf(oseq, oseq.length),
+             seq.getStart(), seq.getEnd());
      description = seq.getDescription();
      if (seq != datasetSequence)
      {
        setDatasetSequence(seq.getDatasetSequence());
      }
-     if (datasetSequence == null && seq.getDBRefs() != null)
+     
+     /*
+      * only copy DBRefs and seqfeatures if we really are a dataset sequence
+      */
+     if (datasetSequence == null)
      {
-       // only copy DBRefs and seqfeatures if we really are a dataset sequence
-       DBRefEntry[] dbr = seq.getDBRefs();
-       for (int i = 0; i < dbr.length; i++)
+       if (seq.getDBRefs() != null)
        {
-         addDBRef(new DBRefEntry(dbr[i]));
+         DBRefEntry[] dbr = seq.getDBRefs();
+         for (int i = 0; i < dbr.length; i++)
+         {
+           addDBRef(new DBRefEntry(dbr[i]));
+         }
        }
        if (seq.getSequenceFeatures() != null)
        {
          }
        }
      }
      if (seq.getAnnotation() != null)
      {
        AlignmentAnnotation[] sqann = seq.getAnnotation();
    @Override
    public synchronized boolean addSequenceFeature(SequenceFeature sf)
    {
 +    if (sf.getType() == null)
 +    {
 +      System.err.println("SequenceFeature type may not be null: "
 +              + sf.toString());
 +      return false;
 +    }
 +
      if (sequenceFeatures == null && datasetSequence != null)
      {
        return datasetSequence.addSequenceFeature(sf);
      temp[sequenceFeatures.length] = sf;
  
      sequenceFeatures = temp;
 +
 +    sequenceFeatureStore.add(sf);
      return true;
    }
  
        return;
      }
  
 +    /*
 +     * new way
 +     */
 +    sequenceFeatureStore.delete(sf);
 +
 +    /*
 +     * old way - to be removed
 +     */
      int index = 0;
      for (index = 0; index < sequenceFeatures.length; index++)
      {
    }
  
    @Override
 +  public SequenceFeaturesI getFeatures()
 +  {
 +    return datasetSequence != null ? datasetSequence.getFeatures()
 +            : sequenceFeatureStore;
 +  }
 +
 +  @Override
    public boolean addPDBId(PDBEntry entry)
    {
      if (pdbIds == null)
        // move features and database references onto dataset sequence
        dsseq.sequenceFeatures = sequenceFeatures;
        sequenceFeatures = null;
 +      dsseq.sequenceFeatureStore = sequenceFeatureStore;
 +      sequenceFeatureStore = null;
        dsseq.dbrefs = dbrefs;
        dbrefs = null;
        // TODO: search and replace any references to this sequence with
        return null;
      }
  
-     Vector subset = new Vector();
-     Enumeration e = annotation.elements();
+     Vector<AlignmentAnnotation> subset = new Vector<AlignmentAnnotation>();
+     Enumeration<AlignmentAnnotation> e = annotation.elements();
      while (e.hasMoreElements())
      {
-       AlignmentAnnotation ann = (AlignmentAnnotation) e.nextElement();
+       AlignmentAnnotation ann = e.nextElement();
        if (ann.label != null && ann.label.equals(label))
        {
          subset.addElement(ann);
      e = subset.elements();
      while (e.hasMoreElements())
      {
-       anns[i++] = (AlignmentAnnotation) e.nextElement();
+       anns[i++] = e.nextElement();
      }
      subset.removeAllElements();
      return anns;
      // transfer PDB entries
      if (entry.getAllPDBEntries() != null)
      {
-       Enumeration e = entry.getAllPDBEntries().elements();
+       Enumeration<PDBEntry> e = entry.getAllPDBEntries().elements();
        while (e.hasMoreElements())
        {
-         PDBEntry pdb = (PDBEntry) e.nextElement();
+         PDBEntry pdb = e.nextElement();
          addPDBId(pdb);
        }
      }
      }
    }
  
 +  /**
 +   * {@inheritDoc}
 +   */
 +  @Override
 +  public List<SequenceFeature> findFeatures(int from, int to,
 +          String... types)
 +  {
 +    if (datasetSequence != null)
 +    {
 +      return datasetSequence.findFeatures(from, to, types);
 +    }
 +    return sequenceFeatureStore.findFeatures(from, to, types);
 +  }
  }
@@@ -23,6 -23,7 +23,7 @@@ 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.assertNotSame;
  import static org.testng.AssertJUnit.assertNull;
  import static org.testng.AssertJUnit.assertSame;
  import static org.testng.AssertJUnit.assertTrue;
@@@ -40,6 -41,6 +41,8 @@@ import java.util.Vector
  
  import junit.extensions.PA;
  
++import junit.extensions.PA;
++
  import org.testng.Assert;
  import org.testng.annotations.BeforeClass;
  import org.testng.annotations.BeforeMethod;
@@@ -287,19 -288,129 +290,129 @@@ public class SequenceTes
    @Test(groups = { "Functional" })
    public void testDeleteChars()
    {
+     /*
+      * internal delete
+      */
+     SequenceI sq = new Sequence("test", "ABCDEF");
+     assertNull(PA.getValue(sq, "datasetSequence"));
+     assertEquals(1, sq.getStart());
+     assertEquals(6, sq.getEnd());
+     sq.deleteChars(2, 3);
+     assertEquals("ABDEF", sq.getSequenceAsString());
+     assertEquals(1, sq.getStart());
+     assertEquals(5, sq.getEnd());
+     assertNull(PA.getValue(sq, "datasetSequence"));
+     /*
+      * delete at start
+      */
+     sq = new Sequence("test", "ABCDEF");
+     sq.deleteChars(0, 2);
+     assertEquals("CDEF", sq.getSequenceAsString());
+     assertEquals(3, sq.getStart());
+     assertEquals(6, sq.getEnd());
+     assertNull(PA.getValue(sq, "datasetSequence"));
+     /*
+      * delete at end
+      */
+     sq = new Sequence("test", "ABCDEF");
+     sq.deleteChars(4, 6);
+     assertEquals("ABCD", sq.getSequenceAsString());
+     assertEquals(1, sq.getStart());
+     assertEquals(4, sq.getEnd());
+     assertNull(PA.getValue(sq, "datasetSequence"));
+   }
+   @Test(groups = { "Functional" })
+   public void testDeleteChars_withDbRefsAndFeatures()
+   {
+     /*
+      * internal delete - new dataset sequence created
+      * gets a copy of any dbrefs
+      */
      SequenceI sq = new Sequence("test", "ABCDEF");
+     sq.createDatasetSequence();
+     DBRefEntry dbr1 = new DBRefEntry("Uniprot", "0", "a123");
+     sq.addDBRef(dbr1);
+     Object ds = PA.getValue(sq, "datasetSequence");
+     assertNotNull(ds);
      assertEquals(1, sq.getStart());
      assertEquals(6, sq.getEnd());
      sq.deleteChars(2, 3);
      assertEquals("ABDEF", sq.getSequenceAsString());
      assertEquals(1, sq.getStart());
      assertEquals(5, sq.getEnd());
+     Object newDs = PA.getValue(sq, "datasetSequence");
+     assertNotNull(newDs);
+     assertNotSame(ds, newDs);
+     assertNotNull(sq.getDBRefs());
+     assertEquals(1, sq.getDBRefs().length);
+     assertNotSame(dbr1, sq.getDBRefs()[0]);
+     assertEquals(dbr1, sq.getDBRefs()[0]);
+     /*
+      * internal delete with sequence features
+      * (failure case for JAL-2541)
+      */
+     sq = new Sequence("test", "ABCDEF");
+     sq.createDatasetSequence();
+     SequenceFeature sf1 = new SequenceFeature("Cath", "desc", 2, 4, 2f,
+             "CathGroup");
+     sq.addSequenceFeature(sf1);
+     ds = PA.getValue(sq, "datasetSequence");
+     assertNotNull(ds);
+     assertEquals(1, sq.getStart());
+     assertEquals(6, sq.getEnd());
+     sq.deleteChars(2, 4);
+     assertEquals("ABEF", sq.getSequenceAsString());
+     assertEquals(1, sq.getStart());
+     assertEquals(4, sq.getEnd());
+     newDs = PA.getValue(sq, "datasetSequence");
+     assertNotNull(newDs);
+     assertNotSame(ds, newDs);
+     SequenceFeature[] sfs = sq.getSequenceFeatures();
+     assertNotNull(sfs);
+     assertEquals(1, sfs.length);
+     assertNotSame(sf1, sfs[0]);
+     assertEquals(sf1, sfs[0]);
  
+     /*
+      * delete at start - no new dataset sequence created
+      * any sequence features remain as before
+      */
      sq = new Sequence("test", "ABCDEF");
+     sq.createDatasetSequence();
+     ds = PA.getValue(sq, "datasetSequence");
+     sf1 = new SequenceFeature("Cath", "desc", 2, 4, 2f, "CathGroup");
+     sq.addSequenceFeature(sf1);
      sq.deleteChars(0, 2);
      assertEquals("CDEF", sq.getSequenceAsString());
      assertEquals(3, sq.getStart());
      assertEquals(6, sq.getEnd());
+     assertSame(ds, PA.getValue(sq, "datasetSequence"));
+     sfs = sq.getSequenceFeatures();
+     assertNotNull(sfs);
+     assertEquals(1, sfs.length);
+     assertSame(sf1, sfs[0]);
+     /*
+      * delete at end - no new dataset sequence created
+      * any dbrefs remain as before
+      */
+     sq = new Sequence("test", "ABCDEF");
+     sq.createDatasetSequence();
+     ds = PA.getValue(sq, "datasetSequence");
+     dbr1 = new DBRefEntry("Uniprot", "0", "a123");
+     sq.addDBRef(dbr1);
+     sq.deleteChars(4, 6);
+     assertEquals("ABCD", sq.getSequenceAsString());
+     assertEquals(1, sq.getStart());
+     assertEquals(4, sq.getEnd());
+     assertSame(ds, PA.getValue(sq, "datasetSequence"));
+     assertNotNull(sq.getDBRefs());
+     assertEquals(1, sq.getDBRefs().length);
+     assertSame(dbr1, sq.getDBRefs()[0]);
    }
  
    @Test(groups = { "Functional" })
      /*
       * SequenceFeature on sequence
       */
 -    SequenceFeature sf = new SequenceFeature();
 +    SequenceFeature sf = new SequenceFeature("Cath", "desc", 2, 4, 2f, null);
      sq.addSequenceFeature(sf);
      SequenceFeature[] sfs = sq.getSequenceFeatures();
      assertEquals(1, sfs.length);
    public void testCreateDatasetSequence()
    {
      SequenceI sq = new Sequence("my", "ASDASD");
 +    sq.addSequenceFeature(new SequenceFeature("type", "desc", 1, 10, 1f,
 +            "group"));
 +    sq.addDBRef(new DBRefEntry("source", "version", "accession"));
      assertNull(sq.getDatasetSequence());
 +    assertNotNull(PA.getValue(sq, "sequenceFeatures")); // to be removed!
 +    assertNotNull(PA.getValue(sq, "sequenceFeatureStore"));
 +    assertNotNull(PA.getValue(sq, "dbrefs"));
 +
      SequenceI rds = sq.createDatasetSequence();
      assertNotNull(rds);
      assertNull(rds.getDatasetSequence());
 -    assertEquals(sq.getDatasetSequence(), rds);
 +    assertSame(sq.getDatasetSequence(), rds);
 +
 +    // sequence features and dbrefs transferred to dataset sequence
 +    assertNull(PA.getValue(sq, "sequenceFeatures"));
 +    assertNull(PA.getValue(sq, "sequenceFeatureStore"));
 +    assertNull(PA.getValue(sq, "dbrefs"));
 +    assertNotNull(PA.getValue(rds, "sequenceFeatures"));
 +    assertNotNull(PA.getValue(rds, "sequenceFeatureStore"));
 +    assertNotNull(PA.getValue(rds, "dbrefs"));
    }
  
    /**
      assertEquals(' ', sq.getCharAt(-1));
    }
  
 +  @Test(groups = { "Functional" })
 +  public void testAddSequenceFeatures()
 +  {
 +    SequenceI sq = new Sequence("", "abcde");
 +    // type may not be null
 +    assertFalse(sq.addSequenceFeature(new SequenceFeature(null, "desc", 4,
 +            8, 0f, null)));
 +    assertTrue(sq.addSequenceFeature(new SequenceFeature("Cath", "desc", 4,
 +            8, 0f, null)));
 +    // can't add a duplicate feature
 +    assertFalse(sq.addSequenceFeature(new SequenceFeature("Cath", "desc",
 +            4, 8, 0f, null)));
 +    // can add a different feature
 +    assertTrue(sq.addSequenceFeature(new SequenceFeature("Scop", "desc", 4,
 +            8, 0f, null))); // different type
 +    assertTrue(sq.addSequenceFeature(new SequenceFeature("Cath",
 +            "description", 4, 8, 0f, null)));// different description
 +    assertTrue(sq.addSequenceFeature(new SequenceFeature("Cath", "desc", 3,
 +            8, 0f, null))); // different start position
 +    assertTrue(sq.addSequenceFeature(new SequenceFeature("Cath", "desc", 4,
 +            9, 0f, null))); // different end position
 +    assertTrue(sq.addSequenceFeature(new SequenceFeature("Cath", "desc", 4,
 +            8, 1f, null))); // different score
 +    assertTrue(sq.addSequenceFeature(new SequenceFeature("Cath", "desc", 4,
 +            8, Float.NaN, null))); // score NaN
 +    assertTrue(sq.addSequenceFeature(new SequenceFeature("Cath", "desc", 4,
 +            8, 0f, "Metal"))); // different group
 +    assertEquals(8, sq.getFeatures().getAllFeatures().size());
 +  }
 +
    /**
     * Tests for adding (or updating) dbrefs
     *