From 6f87303a01399135dc0e673ac41f8fb90991b8de Mon Sep 17 00:00:00 2001
From: gmungoc <g.m.carstairs@dundee.ac.uk>
Date: Mon, 11 Dec 2017 16:09:36 +0000
Subject: [PATCH] JAL-2876 test threshold for Colour by Label

---
 src/jalview/api/FeatureColourI.java         |    9 ++++---
 src/jalview/schemes/FeatureColour.java      |   36 ++++++++++++++-------------
 test/jalview/schemes/FeatureColourTest.java |   29 +++++++++++++++++++++
 3 files changed, 54 insertions(+), 20 deletions(-)

diff --git a/src/jalview/api/FeatureColourI.java b/src/jalview/api/FeatureColourI.java
index 0ded079..18eb008 100644
--- a/src/jalview/api/FeatureColourI.java
+++ b/src/jalview/api/FeatureColourI.java
@@ -146,9 +146,12 @@ public interface FeatureColourI
   boolean hasThreshold();
 
   /**
-   * Returns the computed colour for the given sequence feature. Answers null if
-   * the score of this feature instance is outside the range to render (if any),
-   * i.e. lies below or above a configured threshold.
+   * Returns the computed colour for the given sequence feature. This may be a
+   * simple colour, a colour generated from the feature description (if is
+   * colouring by label), or a colour derived from the feature score (if a
+   * graduated colour).Answers null if the score of this feature instance is
+   * outside the range to render (if any), i.e. lies below or above a configured
+   * threshold.
    * 
    * @param feature
    * @return
diff --git a/src/jalview/schemes/FeatureColour.java b/src/jalview/schemes/FeatureColour.java
index 54d1c6c..17d5d6b 100644
--- a/src/jalview/schemes/FeatureColour.java
+++ b/src/jalview/schemes/FeatureColour.java
@@ -529,10 +529,7 @@ public class FeatureColour implements FeatureColourI
   }
 
   /**
-   * Returns the colour for the given instance of the feature. This may be a
-   * simple colour, a colour generated from the feature description (if
-   * isColourByLabel()), or a colour derived from the feature score (if
-   * isGraduatedColour()).
+   * {@inheritDoc}
    * 
    * @param feature
    * @return
@@ -540,32 +537,37 @@ public class FeatureColour implements FeatureColourI
   @Override
   public Color getColor(SequenceFeature feature)
   {
-    if (isColourByLabel())
-    {
-      return ColorUtils.createColourFromName(feature.getDescription());
-    }
-
-    if (!isGraduatedColour())
+    if (isSimpleColour())
     {
       return getColour();
     }
 
     /*
-     * graduated colour case, optionally with threshold
-     * Float.NaN is assigned minimum visible score colour
+     * return null if score is outwith any threshold
+     * (for graduated colour or colour by label)
      */
     float scr = feature.getScore();
-    if (Float.isNaN(scr))
+    boolean naN = Float.isNaN(scr);
+    if (!naN && isAboveThreshold() && scr <= threshold)
     {
-      return getMinColour();
+      return null;
     }
-    if (isAboveThreshold() && scr <= threshold)
+    if (!naN && isBelowThreshold() && scr >= threshold)
     {
       return null;
     }
-    if (isBelowThreshold() && scr >= threshold)
+
+    if (isColourByLabel())
+    {
+      return ColorUtils.createColourFromName(feature.getDescription());
+    }
+
+    /*
+     * Float.NaN is assigned minimum visible score colour
+     */
+    if (naN)
     {
-      return null;
+      return getMinColour();
     }
     if (range == 0.0)
     {
diff --git a/test/jalview/schemes/FeatureColourTest.java b/test/jalview/schemes/FeatureColourTest.java
index 7a72c15..ed7adb8 100644
--- a/test/jalview/schemes/FeatureColourTest.java
+++ b/test/jalview/schemes/FeatureColourTest.java
@@ -340,4 +340,33 @@ public class FeatureColourTest
     fc = FeatureColour.parseJalviewFeatureColour(descriptor);
     assertTrue(fc.isGraduatedColour());
   }
+
+  @Test(groups = { "Functional" })
+  public void testGetColor_colourByLabel_withThreshold()
+  {
+    FeatureColour fc = new FeatureColour();
+    fc.setColourByLabel(true);
+    SequenceFeature sf = new SequenceFeature("type", "desc", 0, 20, 1f,
+            null);
+    fc.setThreshold(0);
+    Color expected = ColorUtils.createColourFromName("desc");
+    assertEquals(expected, fc.getColor(sf));
+
+    // score (1f) is above threshold
+    fc.setAboveThreshold(true);
+    assertEquals(expected, fc.getColor(sf));
+
+    // score is not above threshold
+    fc.setThreshold(2f);
+    assertNull(fc.getColor(sf));
+
+    // score is not below threshold
+    fc.setThreshold(0f);
+    fc.setBelowThreshold(true);
+    assertNull(fc.getColor(sf));
+
+    // score is below threshold
+    fc.setThreshold(3f);
+    assertEquals(expected, fc.getColor(sf));
+  }
 }
-- 
1.7.10.2