From 6a2fd9a13d79f316acdd4fcff066b8cfaaefa939 Mon Sep 17 00:00:00 2001 From: gmungoc Date: Tue, 18 Apr 2017 10:36:00 +0100 Subject: [PATCH] JAL-2480 test coverage / bug fixes / javadoc --- src/jalview/datamodel/features/FeatureStore.java | 14 +- .../datamodel/features/SequenceFeatures.java | 69 +++--- .../datamodel/features/SequenceFeaturesI.java | 4 +- .../datamodel/features/FeatureStoreTest.java | 72 +++++-- .../datamodel/features/SequenceFeaturesTest.java | 220 ++++++++++++++++++++ 5 files changed, 319 insertions(+), 60 deletions(-) diff --git a/src/jalview/datamodel/features/FeatureStore.java b/src/jalview/datamodel/features/FeatureStore.java index d6a94e2..cb0c36e 100644 --- a/src/jalview/datamodel/features/FeatureStore.java +++ b/src/jalview/datamodel/features/FeatureStore.java @@ -777,12 +777,13 @@ public class FeatureStore } /** - * Answers the number of positional (or non-positional) features stored + * Answers the number of positional (or non-positional) features stored. + * Contact features count as 1. * * @param positional * @return */ - public int size(boolean positional) + public int getFeatureCount(boolean positional) { if (!positional) { @@ -807,14 +808,13 @@ public class FeatureStore } /** - * Answers the average length of positional features (or zero if there are - * none). Contact features contribute a value of 1 to the average. + * Answers the total length of positional features (or zero if there are + * none). Contact features contribute a value of 1 to the total. * * @return */ - public float getAverageFeatureLength() + public int getTotalFeatureLength() { - int d = size(true); - return d == 0 ? 0f : (float) totalExtent / d; + return totalExtent; } } diff --git a/src/jalview/datamodel/features/SequenceFeatures.java b/src/jalview/datamodel/features/SequenceFeatures.java index 4489b1d..4cbf1d9 100644 --- a/src/jalview/datamodel/features/SequenceFeatures.java +++ b/src/jalview/datamodel/features/SequenceFeatures.java @@ -4,7 +4,6 @@ import jalview.datamodel.SequenceFeature; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -37,8 +36,8 @@ public class SequenceFeatures implements SequenceFeaturesI featureStore = new HashMap(); } - /* (non-Javadoc) - * @see jalview.datamodel.features.SequenceFeaturesI#add(jalview.datamodel.SequenceFeature) + /** + * {@inheritDoc} */ @Override public boolean add(SequenceFeature sf) @@ -52,8 +51,8 @@ public class SequenceFeatures implements SequenceFeaturesI return featureStore.get(type).addFeature(sf); } - /* (non-Javadoc) - * @see jalview.datamodel.features.SequenceFeaturesI#findFeatures(int, int, java.lang.String) + /** + * {@inheritDoc} */ @Override public List findFeatures(int from, int to, @@ -73,8 +72,8 @@ public class SequenceFeatures implements SequenceFeaturesI return result; } - /* (non-Javadoc) - * @see jalview.datamodel.features.SequenceFeaturesI#getAllFeatures(java.lang.String) + /** + * {@inheritDoc} */ @Override public List getAllFeatures(String... type) @@ -88,23 +87,27 @@ public class SequenceFeatures implements SequenceFeaturesI return result; } - /* (non-Javadoc) - * @see jalview.datamodel.features.SequenceFeaturesI#getFeatureCount(boolean, java.lang.String) + /** + * {@inheritDoc} */ @Override public int getFeatureCount(boolean positional, String... type) { int result = 0; - for (FeatureStore fs : featureStore.values()) + + for (String featureType : varargToTypes(type)) { - result += fs.size(positional); + FeatureStore featureSet = featureStore.get(featureType); + if (featureSet != null) + { + result += featureSet.getFeatureCount(positional); + } } - return result; } - /* (non-Javadoc) - * @see jalview.datamodel.features.SequenceFeaturesI#getPositionalFeatures(java.lang.String) + /** + * {@inheritDoc} */ @Override public List getPositionalFeatures(String... type) @@ -136,8 +139,8 @@ public class SequenceFeatures implements SequenceFeaturesI .keySet() : Arrays.asList(type); } - /* (non-Javadoc) - * @see jalview.datamodel.features.SequenceFeaturesI#getContactFeatures(java.lang.String) + /** + * {@inheritDoc} */ @Override public List getContactFeatures(String... type) @@ -155,8 +158,8 @@ public class SequenceFeatures implements SequenceFeaturesI return result; } - /* (non-Javadoc) - * @see jalview.datamodel.features.SequenceFeaturesI#getNonPositionalFeatures(java.lang.String) + /** + * {@inheritDoc} */ @Override public List getNonPositionalFeatures(String... type) @@ -174,8 +177,8 @@ public class SequenceFeatures implements SequenceFeaturesI return result; } - /* (non-Javadoc) - * @see jalview.datamodel.features.SequenceFeaturesI#delete(jalview.datamodel.SequenceFeature) + /** + * {@inheritDoc} */ @Override public boolean delete(SequenceFeature sf) @@ -190,8 +193,8 @@ public class SequenceFeatures implements SequenceFeaturesI return false; } - /* (non-Javadoc) - * @see jalview.datamodel.features.SequenceFeaturesI#hasFeatures() + /** + * {@inheritDoc} */ @Override public boolean hasFeatures() @@ -206,8 +209,8 @@ public class SequenceFeatures implements SequenceFeaturesI return false; } - /* (non-Javadoc) - * @see jalview.datamodel.features.SequenceFeaturesI#getFeatureGroups(boolean, java.lang.String) + /** + * {@inheritDoc} */ @Override public Set getFeatureGroups(boolean positionalFeatures, @@ -229,8 +232,8 @@ public class SequenceFeatures implements SequenceFeaturesI return groups; } - /* (non-Javadoc) - * @see jalview.datamodel.features.SequenceFeaturesI#getFeatureTypesForGroups(boolean, java.lang.String) + /** + * {@inheritDoc} */ @Override public Set getFeatureTypesForGroups(boolean positionalFeatures, @@ -258,12 +261,20 @@ public class SequenceFeatures implements SequenceFeaturesI return result; } - /* (non-Javadoc) - * @see jalview.datamodel.features.SequenceFeaturesI#getFeatureTypes() + /** + * {@inheritDoc} */ @Override public Set getFeatureTypes() { - return Collections.unmodifiableSet(featureStore.keySet()); + Set types = new HashSet(); + for (Entry entry : featureStore.entrySet()) + { + if (!entry.getValue().isEmpty()) + { + types.add(entry.getKey()); + } + } + return types; } } diff --git a/src/jalview/datamodel/features/SequenceFeaturesI.java b/src/jalview/datamodel/features/SequenceFeaturesI.java index b6cc3ad..5f904da 100644 --- a/src/jalview/datamodel/features/SequenceFeaturesI.java +++ b/src/jalview/datamodel/features/SequenceFeaturesI.java @@ -116,8 +116,8 @@ public interface SequenceFeaturesI boolean positionalFeatures, String... groups); /** - * Answers an immutable set of the distinct feature types for which a feature - * is stored + * Answers a set of the distinct feature types for which a feature is stored + * * @return */ public abstract Set getFeatureTypes(); diff --git a/test/jalview/datamodel/features/FeatureStoreTest.java b/test/jalview/datamodel/features/FeatureStoreTest.java index b04f810..e4bf122 100644 --- a/test/jalview/datamodel/features/FeatureStoreTest.java +++ b/test/jalview/datamodel/features/FeatureStoreTest.java @@ -365,16 +365,44 @@ public class FeatureStoreTest Float.NaN, null); assertTrue(fs.addFeature(sf1)); - assertEquals(fs.size(true), 1); // positional - assertEquals(fs.size(false), 0); // non-positional + assertEquals(fs.getFeatureCount(true), 1); // positional + assertEquals(fs.getFeatureCount(false), 0); // non-positional /* * re-adding the same or an identical feature should fail */ assertFalse(fs.addFeature(sf1)); - assertEquals(fs.size(true), 1); + assertEquals(fs.getFeatureCount(true), 1); assertFalse(fs.addFeature(sf2)); - assertEquals(fs.size(true), 1); + assertEquals(fs.getFeatureCount(true), 1); + + /* + * add non-positional + */ + SequenceFeature sf3 = new SequenceFeature("Cath", "", 0, 0, Float.NaN, + null); + assertTrue(fs.addFeature(sf3)); + assertEquals(fs.getFeatureCount(true), 1); // positional + assertEquals(fs.getFeatureCount(false), 1); // non-positional + SequenceFeature sf4 = new SequenceFeature("Cath", "", 0, 0, Float.NaN, + null); + assertFalse(fs.addFeature(sf4)); // already stored + assertEquals(fs.getFeatureCount(true), 1); // positional + assertEquals(fs.getFeatureCount(false), 1); // non-positional + + /* + * add contact + */ + SequenceFeature sf5 = new SequenceFeature("Disulfide bond", "", 0, 0, + Float.NaN, null); + assertTrue(fs.addFeature(sf5)); + assertEquals(fs.getFeatureCount(true), 2); // positional - add 1 for contact + assertEquals(fs.getFeatureCount(false), 1); // non-positional + SequenceFeature sf6 = new SequenceFeature("Disulfide bond", "", 0, 0, + Float.NaN, null); + assertFalse(fs.addFeature(sf6)); // already stored + assertEquals(fs.getFeatureCount(true), 2); // no change + assertEquals(fs.getFeatureCount(false), 1); // no change } @Test(groups = "Functional") @@ -382,7 +410,7 @@ public class FeatureStoreTest { FeatureStore fs = new FeatureStore(); assertTrue(fs.isEmpty()); - assertEquals(fs.size(true), 0); + assertEquals(fs.getFeatureCount(true), 0); /* * non-nested feature @@ -391,10 +419,10 @@ public class FeatureStoreTest Float.NaN, null); fs.addFeature(sf1); assertFalse(fs.isEmpty()); - assertEquals(fs.size(true), 1); + assertEquals(fs.getFeatureCount(true), 1); fs.delete(sf1); assertTrue(fs.isEmpty()); - assertEquals(fs.size(true), 0); + assertEquals(fs.getFeatureCount(true), 0); /* * non-positional feature @@ -402,11 +430,11 @@ public class FeatureStoreTest sf1 = new SequenceFeature("Cath", "", 0, 0, Float.NaN, null); fs.addFeature(sf1); assertFalse(fs.isEmpty()); - assertEquals(fs.size(false), 1); // non-positional - assertEquals(fs.size(true), 0); // positional + assertEquals(fs.getFeatureCount(false), 1); // non-positional + assertEquals(fs.getFeatureCount(true), 0); // positional fs.delete(sf1); assertTrue(fs.isEmpty()); - assertEquals(fs.size(false), 0); + assertEquals(fs.getFeatureCount(false), 0); /* * contact feature @@ -414,10 +442,10 @@ public class FeatureStoreTest sf1 = new SequenceFeature("Disulfide bond", "", 19, 49, Float.NaN, null); fs.addFeature(sf1); assertFalse(fs.isEmpty()); - assertEquals(fs.size(true), 1); + assertEquals(fs.getFeatureCount(true), 1); fs.delete(sf1); assertTrue(fs.isEmpty()); - assertEquals(fs.size(true), 0); + assertEquals(fs.getFeatureCount(true), 0); /* * sf2, sf3 added as nested features @@ -430,18 +458,18 @@ public class FeatureStoreTest fs.addFeature(sf1); fs.addFeature(sf2); fs.addFeature(sf3); - assertEquals(fs.size(true), 3); + assertEquals(fs.getFeatureCount(true), 3); assertTrue(fs.delete(sf1)); - assertEquals(fs.size(true), 2); + assertEquals(fs.getFeatureCount(true), 2); // FeatureStore should now only contain features in the NCList assertTrue(fs.nonNestedFeatures.isEmpty()); assertEquals(fs.nestedFeatures.size(), 2); assertFalse(fs.isEmpty()); assertTrue(fs.delete(sf2)); - assertEquals(fs.size(true), 1); + assertEquals(fs.getFeatureCount(true), 1); assertFalse(fs.isEmpty()); assertTrue(fs.delete(sf3)); - assertEquals(fs.size(true), 0); + assertEquals(fs.getFeatureCount(true), 0); assertTrue(fs.isEmpty()); // all gone } @@ -499,27 +527,27 @@ public class FeatureStoreTest } @Test(groups = "Functional") - public void testGetAverageFeatureLength() + public void testGetTotalFeatureLength() { FeatureStore fs = new FeatureStore(); - assertEquals(fs.getAverageFeatureLength(), 0f); + assertEquals(fs.getTotalFeatureLength(), 0); addFeature(fs, 10, 20); // 11 - assertEquals(fs.getAverageFeatureLength(), 11f); + assertEquals(fs.getTotalFeatureLength(), 11); addFeature(fs, 17, 37); // 21 addFeature(fs, 14, 74); // 61 - assertEquals(fs.getAverageFeatureLength(), 31f); + assertEquals(fs.getTotalFeatureLength(), 93); // non-positional features don't count SequenceFeature sf1 = new SequenceFeature("Cath", "desc", 0, 0, 1f, "group1"); fs.addFeature(sf1); - assertEquals(fs.getAverageFeatureLength(), 31f); + assertEquals(fs.getTotalFeatureLength(), 93); // contact features count 1 SequenceFeature sf2 = new SequenceFeature("disulphide bond", "desc", 15, 35, 1f, "group1"); fs.addFeature(sf2); - assertEquals(fs.getAverageFeatureLength(), 94f / 4f); + assertEquals(fs.getTotalFeatureLength(), 94); } } diff --git a/test/jalview/datamodel/features/SequenceFeaturesTest.java b/test/jalview/datamodel/features/SequenceFeaturesTest.java index f9b9b6e..4cd37ee 100644 --- a/test/jalview/datamodel/features/SequenceFeaturesTest.java +++ b/test/jalview/datamodel/features/SequenceFeaturesTest.java @@ -493,4 +493,224 @@ public class SequenceFeaturesTest assertTrue(groups.contains("turn")); assertTrue(groups.contains("strand")); } + + @Test(groups = "Functional") + public void testGetFeatureTypes() + { + SequenceFeaturesI store = new SequenceFeatures(); + Set types = store.getFeatureTypes(); + assertTrue(types.isEmpty()); + + SequenceFeature sf1 = new SequenceFeature("Metal", "desc", 10, 20, + Float.NaN, null); + store.add(sf1); + types = store.getFeatureTypes(); + assertEquals(types.size(), 1); + assertTrue(types.contains("Metal")); + + // null type is possible... + SequenceFeature sf2 = new SequenceFeature(null, "desc", 10, 20, + Float.NaN, null); + store.add(sf2); + types = store.getFeatureTypes(); + assertEquals(types.size(), 2); + assertTrue(types.contains(null)); + assertTrue(types.contains("Metal")); + + /* + * add non-positional feature + */ + SequenceFeature sf3 = new SequenceFeature("Pfam", "desc", 0, 0, + Float.NaN, null); + store.add(sf3); + types = store.getFeatureTypes(); + assertEquals(types.size(), 3); + assertTrue(types.contains("Pfam")); + + /* + * add contact feature + */ + SequenceFeature sf4 = new SequenceFeature("Disulphide Bond", "desc", + 10, 20, Float.NaN, null); + store.add(sf4); + types = store.getFeatureTypes(); + assertEquals(types.size(), 4); + assertTrue(types.contains("Disulphide Bond")); + + /* + * add another Pfam + */ + SequenceFeature sf5 = new SequenceFeature("Pfam", "desc", 10, 20, + Float.NaN, null); + store.add(sf5); + types = store.getFeatureTypes(); + assertEquals(types.size(), 4); // unchanged + + /* + * delete first Pfam - still have one + */ + assertTrue(store.delete(sf3)); + types = store.getFeatureTypes(); + assertEquals(types.size(), 4); + assertTrue(types.contains("Pfam")); + + /* + * delete second Pfam - no longer have one + */ + assertTrue(store.delete(sf5)); + types = store.getFeatureTypes(); + assertEquals(types.size(), 3); + assertFalse(types.contains("Pfam")); + } + + @Test(groups = "Functional") + public void testGetFeatureCount() + { + SequenceFeaturesI store = new SequenceFeatures(); + assertEquals(store.getFeatureCount(true), 0); + assertEquals(store.getFeatureCount(false), 0); + + /* + * add positional + */ + SequenceFeature sf1 = new SequenceFeature("Metal", "desc", 10, 20, + Float.NaN, null); + store.add(sf1); + assertEquals(store.getFeatureCount(true), 1); + assertEquals(store.getFeatureCount(false), 0); + + /* + * another positional + */ + SequenceFeature sf2 = new SequenceFeature(null, "desc", 10, 20, + Float.NaN, null); + store.add(sf2); + assertEquals(store.getFeatureCount(true), 2); + assertEquals(store.getFeatureCount(false), 0); + + /* + * add non-positional feature + */ + SequenceFeature sf3 = new SequenceFeature("Pfam", "desc", 0, 0, + Float.NaN, null); + store.add(sf3); + assertEquals(store.getFeatureCount(true), 2); + assertEquals(store.getFeatureCount(false), 1); + + /* + * add contact feature (counts as 1) + */ + SequenceFeature sf4 = new SequenceFeature("Disulphide Bond", "desc", + 10, 20, Float.NaN, null); + store.add(sf4); + assertEquals(store.getFeatureCount(true), 3); + assertEquals(store.getFeatureCount(false), 1); + + /* + * add another Pfam + */ + SequenceFeature sf5 = new SequenceFeature("Pfam", "desc", 10, 20, + Float.NaN, null); + store.add(sf5); + assertEquals(store.getFeatureCount(true), 4); + assertEquals(store.getFeatureCount(false), 1); + assertEquals(store.getFeatureCount(true, "Pfam"), 1); + assertEquals(store.getFeatureCount(false, "Pfam"), 1); + // search for type==null + assertEquals(store.getFeatureCount(true, (String) null), 1); + // search with no type specified + assertEquals(store.getFeatureCount(true, (String[]) null), 4); + assertEquals(store.getFeatureCount(true, "Metal", "Cath"), 1); + assertEquals(store.getFeatureCount(true, "Disulphide Bond"), 1); + assertEquals(store.getFeatureCount(true, "Metal", "Pfam", null), 3); + + /* + * delete first Pfam (non-positional) + */ + assertTrue(store.delete(sf3)); + assertEquals(store.getFeatureCount(true), 4); + assertEquals(store.getFeatureCount(false), 0); + + /* + * delete second Pfam (positional) + */ + assertTrue(store.delete(sf5)); + assertEquals(store.getFeatureCount(true), 3); + assertEquals(store.getFeatureCount(false), 0); + } + + @Test(groups = "Functional") + public void testGetAllFeatures() + { + SequenceFeaturesI store = new SequenceFeatures(); + List features = store.getAllFeatures(); + assertTrue(features.isEmpty()); + + SequenceFeature sf1 = new SequenceFeature("Metal", "desc", 10, 20, + Float.NaN, null); + store.add(sf1); + features = store.getAllFeatures(); + assertEquals(features.size(), 1); + assertTrue(features.contains(sf1)); + + SequenceFeature sf2 = new SequenceFeature(null, "desc", 10, 20, + Float.NaN, null); + store.add(sf2); + features = store.getAllFeatures(); + assertEquals(features.size(), 2); + assertTrue(features.contains(sf2)); + + /* + * add non-positional feature + */ + SequenceFeature sf3 = new SequenceFeature("Pfam", "desc", 0, 0, + Float.NaN, null); + store.add(sf3); + features = store.getAllFeatures(); + assertEquals(features.size(), 3); + assertTrue(features.contains(sf3)); + + /* + * add contact feature + */ + SequenceFeature sf4 = new SequenceFeature("Disulphide Bond", "desc", + 10, 20, Float.NaN, null); + store.add(sf4); + features = store.getAllFeatures(); + assertEquals(features.size(), 4); + assertTrue(features.contains(sf4)); + + /* + * add another Pfam + */ + SequenceFeature sf5 = new SequenceFeature("Pfam", "desc", 10, 20, + Float.NaN, null); + store.add(sf5); + features = store.getAllFeatures(); + assertEquals(features.size(), 5); + assertTrue(features.contains(sf5)); + features = store.getAllFeatures("Cath"); + assertTrue(features.isEmpty()); + features = store.getAllFeatures("Pfam", "Cath", "Metal"); + assertEquals(features.size(), 3); + assertTrue(features.contains(sf1)); + assertTrue(features.contains(sf3)); + assertTrue(features.contains(sf5)); + + /* + * delete first Pfam + */ + assertTrue(store.delete(sf3)); + features = store.getAllFeatures(); + assertEquals(features.size(), 4); + assertFalse(features.contains(sf3)); + + /* + * delete second Pfam + */ + assertTrue(store.delete(sf5)); + features = store.getAllFeatures(); + assertEquals(features.size(), 3); + assertFalse(features.contains(sf3)); + } } -- 1.7.10.2