From: gmungoc <g.m.carstairs@dundee.ac.uk>
Date: Wed, 24 May 2017 14:21:43 +0000 (+0100)
Subject: Merge branch 'bug/JAL-2541cutWithFeatures' into features/JAL-2446NCList
X-Git-Tag: Release_2_10_3b1~239
X-Git-Url: http://source.jalview.org/gitweb/?a=commitdiff_plain;h=f1fbc7674102f63dfe1bd156a2d19f3c658e35d5;hp=-c;p=jalview.git

Merge branch 'bug/JAL-2541cutWithFeatures' into features/JAL-2446NCList
---

f1fbc7674102f63dfe1bd156a2d19f3c658e35d5
diff --combined src/jalview/commands/EditCommand.java
index 98ac2d5,21ff841..388c533
--- a/src/jalview/commands/EditCommand.java
+++ b/src/jalview/commands/EditCommand.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,
@@@ -1101,8 -1101,8 +1102,8 @@@
      }
    }
  
--  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();
@@@ -1122,69 -1122,51 +1123,69 @@@
        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)
diff --combined src/jalview/datamodel/Sequence.java
index af6592b,9994675..9f3e7b8
--- a/src/jalview/datamodel/Sequence.java
+++ b/src/jalview/datamodel/Sequence.java
@@@ -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;
  
  /**
@@@ -47,6 -47,11 +49,11 @@@
   */
  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.
     * 
@@@ -124,16 -127,10 +131,11 @@@
      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)
@@@ -235,23 -232,28 +237,28 @@@
    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)
        {
@@@ -262,6 -264,7 +269,7 @@@
          }
        }
      }
+ 
      if (seq.getAnnotation() != null)
      {
        AlignmentAnnotation[] sqann = seq.getAnnotation();
@@@ -321,13 -324,6 +329,13 @@@
    @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);
@@@ -350,8 -346,6 +358,8 @@@
      temp[sequenceFeatures.length] = sf;
  
      sequenceFeatures = temp;
 +
 +    sequenceFeatureStore.add(sf);
      return true;
    }
  
@@@ -367,14 -361,6 +375,14 @@@
        return;
      }
  
 +    /*
 +     * new way
 +     */
 +    sequenceFeatureStore.delete(sf);
 +
 +    /*
 +     * old way - to be removed
 +     */
      int index = 0;
      for (index = 0; index < sequenceFeatures.length; index++)
      {
@@@ -434,13 -420,6 +442,13 @@@
    }
  
    @Override
 +  public SequenceFeaturesI getFeatures()
 +  {
 +    return datasetSequence != null ? datasetSequence.getFeatures()
 +            : sequenceFeatureStore;
 +  }
 +
 +  @Override
    public boolean addPDBId(PDBEntry entry)
    {
      if (pdbIds == null)
@@@ -1185,8 -1164,6 +1193,8 @@@
        // 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
@@@ -1245,11 -1222,11 +1253,11 @@@
        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);
@@@ -1264,7 -1241,7 +1272,7 @@@
      e = subset.elements();
      while (e.hasMoreElements())
      {
-       anns[i++] = (AlignmentAnnotation) e.nextElement();
+       anns[i++] = e.nextElement();
      }
      subset.removeAllElements();
      return anns;
@@@ -1335,10 -1312,10 +1343,10 @@@
      // 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);
        }
      }
@@@ -1506,17 -1483,4 +1514,17 @@@
      }
    }
  
 +  /**
 +   * {@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);
 +  }
  }
diff --combined test/jalview/datamodel/SequenceTest.java
index 586cc5d,739ef5d..ebf4857
--- a/test/jalview/datamodel/SequenceTest.java
+++ b/test/jalview/datamodel/SequenceTest.java
@@@ -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" })
@@@ -342,7 -453,7 +455,7 @@@
      /*
       * 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);
@@@ -437,26 -548,11 +550,26 @@@
    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"));
    }
  
    /**
@@@ -728,36 -824,6 +841,36 @@@
      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
     *