From 0a461d38627bf41323b8f77e5c6f45b300025494 Mon Sep 17 00:00:00 2001 From: gmungoc Date: Thu, 20 Apr 2017 16:44:50 +0100 Subject: [PATCH] JAL-2483 JAL-2481 findAllFeatures without scanning all features --- .../datamodel/features/SequenceFeatures.java | 18 +-- .../datamodel/features/SequenceFeaturesI.java | 47 ++++-- .../seqfeatures/FeatureRendererModel.java | 159 +++++++++++--------- .../datamodel/features/SequenceFeaturesTest.java | 2 +- .../renderer/seqfeatures/FeatureRendererTest.java | 133 ++++++++++++++++ 5 files changed, 260 insertions(+), 99 deletions(-) create mode 100644 test/jalview/renderer/seqfeatures/FeatureRendererTest.java diff --git a/src/jalview/datamodel/features/SequenceFeatures.java b/src/jalview/datamodel/features/SequenceFeatures.java index c825761..c6af3ea 100644 --- a/src/jalview/datamodel/features/SequenceFeatures.java +++ b/src/jalview/datamodel/features/SequenceFeatures.java @@ -299,14 +299,9 @@ public class SequenceFeatures implements SequenceFeaturesI } /** - * Answers the minimum score held for positional or non-positional features - * for the specified type. This may be Float.NaN if there are no features, or - * none has a non-NaN score. - * - * @param type - * @param positional - * @return + * {@inheritDoc} */ + @Override public float getMinimumScore(String type, boolean positional) { return featureStore.containsKey(type) ? featureStore.get(type) @@ -314,14 +309,9 @@ public class SequenceFeatures implements SequenceFeaturesI } /** - * Answers the maximum score held for positional or non-positional features - * for the specified type. This may be Float.NaN if there are no features, or - * none has a non-NaN score. - * - * @param type - * @param positional - * @return + * {@inheritDoc} */ + @Override public float getMaximumScore(String type, boolean positional) { return featureStore.containsKey(type) ? featureStore.get(type) diff --git a/src/jalview/datamodel/features/SequenceFeaturesI.java b/src/jalview/datamodel/features/SequenceFeaturesI.java index 43e9448..fa77532 100644 --- a/src/jalview/datamodel/features/SequenceFeaturesI.java +++ b/src/jalview/datamodel/features/SequenceFeaturesI.java @@ -16,7 +16,7 @@ public interface SequenceFeaturesI * * @param sf */ - abstract boolean add(SequenceFeature sf); + boolean add(SequenceFeature sf); /** * Returns a (possibly empty) list of features, optionally restricted to @@ -28,7 +28,7 @@ public interface SequenceFeaturesI * @param type * @return */ - abstract List findFeatures(int from, int to, + List findFeatures(int from, int to, String... type); /** @@ -38,7 +38,7 @@ public interface SequenceFeaturesI * @param type * @return */ - abstract List getAllFeatures(String... type); + List getAllFeatures(String... type); /** * Answers the number of (positional or non-positional) features, optionally @@ -48,7 +48,7 @@ public interface SequenceFeaturesI * @param type * @return */ - abstract int getFeatureCount(boolean positional, String... type); + int getFeatureCount(boolean positional, String... type); /** * Answers the total length of positional features, optionally restricted to @@ -57,7 +57,7 @@ public interface SequenceFeaturesI * @param type * @return */ - abstract int getTotalFeatureLength(String... type); + int getTotalFeatureLength(String... type); /** * Answers a list of all positional features, optionally restricted to @@ -66,7 +66,7 @@ public interface SequenceFeaturesI * @param type * @return */ - abstract List getPositionalFeatures( + List getPositionalFeatures( String... type); /** @@ -75,7 +75,7 @@ public interface SequenceFeaturesI * * @return */ - abstract List getContactFeatures(String... type); + List getContactFeatures(String... type); /** * Answers a list of all non-positional features, optionally restricted to @@ -85,7 +85,7 @@ public interface SequenceFeaturesI * if no type is specified, all are returned * @return */ - abstract List getNonPositionalFeatures( + List getNonPositionalFeatures( String... type); /** @@ -96,14 +96,14 @@ public interface SequenceFeaturesI * * @param sf */ - abstract boolean delete(SequenceFeature sf); + boolean delete(SequenceFeature sf); /** * Answers true if this store contains at least one feature, else false * * @return */ - abstract boolean hasFeatures(); + boolean hasFeatures(); /** * Returns a set of the distinct feature groups present in the collection. The @@ -116,7 +116,7 @@ public interface SequenceFeaturesI * @param type * @return */ - abstract Set getFeatureGroups(boolean positionalFeatures, + Set getFeatureGroups(boolean positionalFeatures, String... type); /** @@ -129,7 +129,7 @@ public interface SequenceFeaturesI * @param groups * @return */ - abstract Set getFeatureTypesForGroups( + Set getFeatureTypesForGroups( boolean positionalFeatures, String... groups); /** @@ -137,6 +137,27 @@ public interface SequenceFeaturesI * * @return */ - abstract Set getFeatureTypes(); + Set getFeatureTypes(); + /** + * Answers the minimum score held for positional or non-positional features + * for the specified type. This may be Float.NaN if there are no features, or + * none has a non-NaN score. + * + * @param type + * @param positional + * @return + */ + float getMinimumScore(String type, boolean positional); + + /** + * Answers the maximum score held for positional or non-positional features + * for the specified type. This may be Float.NaN if there are no features, or + * none has a non-NaN score. + * + * @param type + * @param positional + * @return + */ + float getMaximumScore(String type, boolean positional); } \ No newline at end of file diff --git a/src/jalview/viewmodel/seqfeatures/FeatureRendererModel.java b/src/jalview/viewmodel/seqfeatures/FeatureRendererModel.java index 68febb9..4ab050d 100644 --- a/src/jalview/viewmodel/seqfeatures/FeatureRendererModel.java +++ b/src/jalview/viewmodel/seqfeatures/FeatureRendererModel.java @@ -35,7 +35,9 @@ import java.beans.PropertyChangeListener; import java.beans.PropertyChangeSupport; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.Hashtable; import java.util.Iterator; import java.util.List; @@ -306,7 +308,7 @@ public abstract class FeatureRendererModel implements * Searches alignment for all features and updates colours * * @param newMadeVisible - * if true newly added feature types will be rendered immediatly + * if true newly added feature types will be rendered immediately * TODO: check to see if this method should actually be proxied so * repaint events can be propagated by the renderer code */ @@ -328,8 +330,7 @@ public abstract class FeatureRendererModel implements } FeaturesDisplayedI featuresDisplayed = av.getFeaturesDisplayed(); - ArrayList allfeatures = new ArrayList(); - ArrayList oldfeatures = new ArrayList(); + Set oldfeatures = new HashSet(); if (renderOrder != null) { for (int i = 0; i < renderOrder.length; i++) @@ -340,94 +341,109 @@ public abstract class FeatureRendererModel implements } } } - if (minmax == null) - { - minmax = new Hashtable(); - } + AlignmentI alignment = av.getAlignment(); + List allfeatures = new ArrayList(); // or HashSet? + for (int i = 0; i < alignment.getHeight(); i++) { SequenceI asq = alignment.getSequenceAt(i); - SequenceFeature[] features = asq.getSequenceFeatures(); - - if (features == null) + for (String group : asq.getFeatures().getFeatureGroups(true)) { - continue; - } - - int index = 0; - while (index < features.length) - { - if (!featuresDisplayed.isRegistered(features[index].getType())) + if (group == null) { - String fgrp = features[index].getFeatureGroup(); - if (fgrp != null) - { - Boolean groupDisplayed = featureGroups.get(fgrp); - if (groupDisplayed == null) - { - groupDisplayed = Boolean.valueOf(newMadeVisible); - featureGroups.put(fgrp, groupDisplayed); - } - if (!groupDisplayed.booleanValue()) - { - index++; - continue; - } - } - if (!(features[index].begin == 0 && features[index].end == 0)) - { - // If beginning and end are 0, the feature is for the whole sequence - // and we don't want to render the feature in the normal way - - if (newMadeVisible - && !oldfeatures.contains(features[index].getType())) - { - // this is a new feature type on the alignment. Mark it for - // display. - featuresDisplayed.setVisible(features[index].getType()); - setOrder(features[index].getType(), 0); - } - } + continue; } - if (!allfeatures.contains(features[index].getType())) + Boolean groupDisplayed = featureGroups.get(group); + if (groupDisplayed == null) { - allfeatures.add(features[index].getType()); + groupDisplayed = Boolean.valueOf(newMadeVisible); + featureGroups.put(group, groupDisplayed); } - if (!Float.isNaN(features[index].score)) + if (groupDisplayed) { - int nonpos = features[index].getBegin() >= 1 ? 0 : 1; - float[][] mm = minmax.get(features[index].getType()); - if (mm == null) - { - mm = new float[][] { null, null }; - minmax.put(features[index].getType(), mm); - } - if (mm[nonpos] == null) - { - mm[nonpos] = new float[] { features[index].score, - features[index].score }; - - } - else + Set types = asq.getFeatures().getFeatureTypesForGroups( + true, group); + for (String type : types) { - if (mm[nonpos][0] > features[index].score) - { - mm[nonpos][0] = features[index].score; - } - if (mm[nonpos][1] < features[index].score) + if (!allfeatures.contains(type)) // or use HashSet and no test? { - mm[nonpos][1] = features[index].score; + allfeatures.add(type); } + updateMinMax(asq, type, true); // todo: for all features? } } - index++; } } + + /* + * mark any new feature types as visible + */ + Collections.sort(allfeatures, String.CASE_INSENSITIVE_ORDER); + if (newMadeVisible) + { + for (String type : allfeatures) + { + if (!oldfeatures.contains(type)) + { + featuresDisplayed.setVisible(type); + setOrder(type, 0); + } + } + } + updateRenderOrder(allfeatures); findingFeatures = false; } + /** + * Updates the global (alignment) min and max values for a feature type from + * the score for a sequence, if the score is not NaN. Values are stored + * separately for positional and non-positional features. + * + * @param seq + * @param featureType + * @param positional + */ + protected void updateMinMax(SequenceI seq, String featureType, + boolean positional) + { + float min = seq.getFeatures().getMinimumScore(featureType, positional); + if (Float.isNaN(min)) + { + return; + } + + float max = seq.getFeatures().getMaximumScore(featureType, positional); + + /* + * stored values are + * { {positionalMin, positionalMax}, {nonPositionalMin, nonPositionalMax} } + */ + if (minmax == null) + { + minmax = new Hashtable(); + } + synchronized (minmax) + { + float[][] mm = minmax.get(featureType); + int index = positional ? 0 : 1; + if (mm == null) + { + mm = new float[][] { null, null }; + minmax.put(featureType, mm); + } + if (mm[index] == null) + { + mm[index] = new float[] { min, max }; + } + else + { + mm[index][0] = Math.min(mm[index][0], min); + mm[index][1] = Math.max(mm[index][1], max); + } + } + } protected Boolean firing = Boolean.FALSE; /** @@ -664,7 +680,8 @@ public abstract class FeatureRendererModel implements } /** - * Sets the priority order for features + * Sets the priority order for features, with the highest priority (displayed + * on top) at the start of the data array * * @param data * { String(Type), Colour(Type), Boolean(Displayed) } diff --git a/test/jalview/datamodel/features/SequenceFeaturesTest.java b/test/jalview/datamodel/features/SequenceFeaturesTest.java index eaf20b0..bb11a87 100644 --- a/test/jalview/datamodel/features/SequenceFeaturesTest.java +++ b/test/jalview/datamodel/features/SequenceFeaturesTest.java @@ -769,7 +769,7 @@ public class SequenceFeaturesTest assertEquals(store.getTotalFeatureLength(), 0); } - @Test + @Test(groups = "Functional") public void testGetMinimumScore_getMaximumScore() { SequenceFeatures sf = new SequenceFeatures(); diff --git a/test/jalview/renderer/seqfeatures/FeatureRendererTest.java b/test/jalview/renderer/seqfeatures/FeatureRendererTest.java new file mode 100644 index 0000000..febd306 --- /dev/null +++ b/test/jalview/renderer/seqfeatures/FeatureRendererTest.java @@ -0,0 +1,133 @@ +package jalview.renderer.seqfeatures; + +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertTrue; + +import jalview.api.AlignViewportI; +import jalview.api.FeatureColourI; +import jalview.datamodel.SequenceFeature; +import jalview.datamodel.SequenceI; +import jalview.gui.AlignFrame; +import jalview.io.DataSourceType; +import jalview.io.FileLoader; +import jalview.schemes.FeatureColour; + +import java.awt.Color; +import java.util.Arrays; +import java.util.List; +import java.util.Map; + +import org.testng.annotations.Test; + +public class FeatureRendererTest +{ + + @Test(groups = "Functional") + public void testFindAllFeatures() + { + String seqData = ">s1\nabcdef\n>s2\nabcdef\n>s3\nabcdef\n>s4\nabcdef\n"; + AlignFrame af = new FileLoader().LoadFileWaitTillLoaded(seqData, + DataSourceType.PASTE); + AlignViewportI av = af.getViewport(); + FeatureRenderer fr = new FeatureRenderer(av); + + /* + * with no features + */ + fr.findAllFeatures(true); + assertTrue(fr.getRenderOrder().isEmpty()); + assertTrue(fr.getFeatureGroups().isEmpty()); + + List seqs = av.getAlignment().getSequences(); + + // add a non-positional feature - should be ignored by FeatureRenderer + SequenceFeature sf1 = new SequenceFeature("Type", "Desc", 0, 0, 1f, + "Group"); + seqs.get(0).addSequenceFeature(sf1); + fr.findAllFeatures(true); + // ? bug - types and groups added for non-positional features + List types = fr.getRenderOrder(); + List groups = fr.getFeatureGroups(); + assertEquals(types.size(), 0); + assertFalse(types.contains("Type")); + assertEquals(groups.size(), 0); + assertFalse(groups.contains("Group")); + + // add some positional features + seqs.get(1).addSequenceFeature( + new SequenceFeature("Pfam", "Desc", 5, 9, 1f, "PfamGroup")); + seqs.get(2).addSequenceFeature( + new SequenceFeature("Pfam", "Desc", 14, 22, 2f, "RfamGroup")); + // bug in findAllFeatures - group not checked for a known feature type + seqs.get(2).addSequenceFeature( + new SequenceFeature("Rfam", "Desc", 5, 9, Float.NaN, + "RfamGroup")); + seqs.get(3).addSequenceFeature( + new SequenceFeature("Rfam", "Desc", 5, 9, Float.NaN, null)); + // null value for type produces NullPointerException + fr.findAllFeatures(true); + types = fr.getRenderOrder(); + groups = fr.getFeatureGroups(); + assertEquals(types.size(), 2); + assertFalse(types.contains("Type")); + assertTrue(types.contains("Pfam")); + assertTrue(types.contains("Rfam")); + assertEquals(groups.size(), 2); + assertFalse(groups.contains("Group")); + assertTrue(groups.contains("PfamGroup")); + assertTrue(groups.contains("RfamGroup")); + assertFalse(groups.contains(null)); // null group is ignored + + /* + * check min-max values + */ + Map minMax = fr.getMinMax(); + assertEquals(minMax.size(), 1); // non-positional and NaN not stored + assertEquals(minMax.get("Pfam")[0][0], 1f); // positional min + assertEquals(minMax.get("Pfam")[0][1], 2f); // positional max + + // increase max for Pfam, add scores for Rfam + seqs.get(0).addSequenceFeature( + new SequenceFeature("Pfam", "Desc", 14, 22, 8f, "RfamGroup")); + seqs.get(1).addSequenceFeature( + new SequenceFeature("Rfam", "Desc", 5, 9, 6f, "RfamGroup")); + fr.findAllFeatures(true); + // note minMax is not a defensive copy, shouldn't expose this + assertEquals(minMax.size(), 2); + assertEquals(minMax.get("Pfam")[0][0], 1f); + assertEquals(minMax.get("Pfam")[0][1], 8f); + assertEquals(minMax.get("Rfam")[0][0], 6f); + assertEquals(minMax.get("Rfam")[0][1], 6f); + + /* + * check render order (last is on top) + */ + List renderOrder = fr.getRenderOrder(); + assertEquals(renderOrder, Arrays.asList("Rfam", "Pfam")); + + /* + * change render order (todo: an easier way) + * nb here last comes first in the data array + */ + Object[][] data = new Object[2][]; + FeatureColourI colour = new FeatureColour(Color.RED); + data[0] = new Object[] { "Rfam", colour, true }; + data[1] = new Object[] { "Pfam", colour, false }; + fr.setFeaturePriority(data); + assertEquals(fr.getRenderOrder(), Arrays.asList("Pfam", "Rfam")); + assertEquals(fr.getDisplayedFeatureTypes(), Arrays.asList("Rfam")); + + /* + * add a new feature type: should go on top of render order as visible, + * other feature ordering and visibility should be unchanged + */ + seqs.get(2).addSequenceFeature( + new SequenceFeature("Metal", "Desc", 14, 22, 8f, "MetalGroup")); + fr.findAllFeatures(true); + assertEquals(fr.getRenderOrder(), + Arrays.asList("Pfam", "Rfam", "Metal")); + assertEquals(fr.getDisplayedFeatureTypes(), + Arrays.asList("Rfam", "Metal")); + } +} -- 1.7.10.2