Fix color sorting in the legend

.travis.yml

@@ -44,7 +44,7 @@ install:
   - npm install -g jsdoc
 
 script:
-  - flake8 emperor/*.py tests/*.py scripts/*.py setup.py
+  - flake8 emperor/*.py tests/*.py setup.py
   # we just check coverage in the latest version of NumPy
   - coverage run tests/all_tests.py && coverage report
   - make -C doc html

ChangeLog.md

@@ -4,6 +4,15 @@ Emperor ChangeLog
 # Emperor 1.0.0-dev (changes since 1.0.0 go here)
 --------------------------------------------------
 
+### Bug Fixes
+* Fix "incomplete" interpolation when using sequential / diverging color maps
+  (e.g. `viridis`) and when not using the `Continuous values` option
+  ([#760](https://github.com/biocore/emperor/issues/760))
+* Make the order consistent between color assignment and sorting in the legend
+  ([#761](https://github.com/biocore/emperor/issues/761))
+* Update the Chroma.js version to v2.1.0 from v1.1.1; this resulted in some
+  very slight precision differences in things like color interpolation
+  ([#762](https://github.com/biocore/emperor/issues/762))
 
 # Emperor 1.0.0
 ---------------

emperor/support_files/js/view.js

@@ -4,8 +4,9 @@ define([
     'three',
     'shapes',
     'draw',
-    'multi-model'
-], function($, _, THREE, shapes, draw, multiModel) {
+    'multi-model',
+    'util'
+], function($, _, THREE, shapes, draw, multiModel, util) {
   var makeArrow = draw.makeArrow;
   var makeLineCollection = draw.makeLineCollection;
 /**
@@ -769,24 +770,25 @@ DecompositionView.prototype.flipVisibleDimension = function(index) {
 DecompositionView.prototype.setCategory = function(attributes,
                                                    setPlottableAttributes,
                                                    category) {
-  var scope = this, dataView = [], plottables, i = 0;
+  var scope = this, dataView = [], plottables;
 
-  _.each(attributes, function(value, key) {
+  var fieldValues = util.naturalSort(_.keys(attributes));
+
+  _.each(fieldValues, function(fieldVal, index) {
     /*
      *
      * WARNING: This is mixing attributes of the view with the model ...
      * it's a bit of a gray area though.
      *
      **/
     plottables = scope.decomp.getPlottablesByMetadataCategoryValue(category,
-                                                                   key);
+                                                                   fieldVal);
     if (setPlottableAttributes !== null) {
-      setPlottableAttributes(scope, value, plottables);
+      setPlottableAttributes(scope, attributes[fieldVal], plottables);
     }
 
-    dataView.push({id: i, category: key, value: value,
+    dataView.push({id: index, category: fieldVal, value: attributes[fieldVal],
                    plottables: plottables});
-    i = i + 1;
   });
   this.needsUpdate = true;
 

tests/javascript_tests/test_color_view_controller.js

@@ -481,8 +481,8 @@ requirejs([
       );
       // Test that extreme values are correctly assigned to the colors at the
       // ends of the BrBG color map
-      deepEqual(interpolatedColorList[0]["0"], "#543005");
-      deepEqual(interpolatedColorList[0]["19"], "#003c30");
+      deepEqual(interpolatedColorList[0]['0'], '#543005');
+      deepEqual(interpolatedColorList[0]['19'], '#003c30');
       // Now, check that all values (incl. intermediate ones) are correctly
       // assigned colors
       var interpolator = chroma.scale(chroma.brewer.BrBG).domain([0, 19]);

tests/javascript_tests/test_decomposition_view.js

@@ -145,7 +145,6 @@ requirejs([
                        '20071112']];
       decomp = new DecompositionModel(data, md_headers, metadata);
       var mm = new MultiModel({'scatter': decomp});
-      var obs;
 
       var UIState1 = new UIState();
       UIState1.setProperty('view.usesPointCloud', false);
@@ -245,6 +244,86 @@ requirejs([
       deepEqual(view.lines.right.geometry.attributes.position.array.length, 6);
     });
 
+    /**
+     *
+     * Test sorting of field values, so that we don't accidentally regress
+     * on the issue described at https://github.com/biocore/emperor/issues/761.
+     *
+     * The boilerplate code in this test is mostly just copied from the "Test
+     * constructor with two dimensions" test above.
+     *
+     */
+    test('Test that setCategory sorts field values properly', function() {
+
+      var data = {
+        name: 'pcoa',
+        sample_ids: ['s1', 's2', 's3', 's4', 's5', 's6', 's7'],
+        coordinates: [
+            [-0.25, 0.25],
+            [0, 0.25],
+            [0.25, 0.25],
+            [0.25, -0.25],
+            [0, -0.25],
+            [-0.25, -0.25],
+            [0, 0]
+        ],
+        percents_explained: [80.0, 20.0]
+      };
+      var md_headers = ['SampleID', 'DeliberatelyAnnoyingField'];
+      var metadata = [
+          ['s1', '3'],
+          ['s2', ':('],
+          ['s3', 'NotANumber'],
+          ['s4', 'Four'],
+          ['s5', '-5'],
+          ['s6', '6'],
+          ['s7', ':(']
+      ];
+      var decomp = new DecompositionModel(data, md_headers, metadata);
+      var mm = new MultiModel({'scatter': decomp});
+
+      var UIState1 = new UIState();
+      UIState1.setProperty('view.usesPointCloud', false);
+      var dv = new DecompositionView(mm, 'scatter', UIState1);
+
+      // This is the "data" we'll be providing to setCategory(). This is
+      // analogous to what the "categorySelectionCallback" in the
+      // ColorViewController would pass to setCategory(): it maps unique
+      // metadata categories to colors. (The "unique" is why there are only 6
+      // entries in this, since s2 and s7 share a metadata value for
+      // DeliberatelyAnnoyingField.)
+      // Anyway, the jsonData object has been purposely shuffled to be
+      // "out of order" (... I guess Objects don't really have an order,
+      // but you get the idea.)
+      var jsonData = {
+        '-5': '#008000',
+        '3': '#91278d',
+        '6': '#ffff00',
+        ':(': '#ff0000',
+        'Four': '#0000ff',
+        'NotANumber': '#f27304',
+      };
+
+      var expectedMetadataOrder = [':(', 'Four', 'NotANumber', '-5', '3', '6'];
+      // The colors I've used for this test are just the 'Classic QIIME Colors'
+      var expectedColorOrder = [
+        '#ff0000', '#0000ff', '#f27304', '#008000', '#91278d', '#ffff00'
+      ];
+
+      var dataView = dv.setCategory(
+          jsonData, null, 'DeliberatelyAnnoyingField'
+      );
+
+      // go through dataView and check that the metadata categories are in
+      // the correct order (as defined by expectedMetadataOrder)
+      _.each(dataView, function(fieldObject, index) {
+        deepEqual(fieldObject.category, expectedMetadataOrder[index]);
+        deepEqual(fieldObject.id, index);
+        // While we're at it, check that the colors are correct
+        deepEqual(fieldObject.value, expectedColorOrder[index]);
+      });
+    });
+
     test('Test constructor fails (biplot in fast mode)', function() {
       this.decomp.type = 'arrow';
       throws(function() {
@@ -319,7 +398,7 @@ requirejs([
       UIState1.setProperty('view.usesPointCloud', false);
       var dv = new DecompositionView(this.multiModel, 'scatter', UIState1);
       dv.changeVisibleDimensions([2, 3, 4]);
-      obs = [dv.markers[0].position.x,
+      var obs = [dv.markers[0].position.x,
       dv.markers[0].position.y,
       dv.markers[0].position.z];
       exp = [0.066647, -0.067711, 0.176070];
@@ -346,7 +425,7 @@ requirejs([
                                      UIState1);
       dv.changeVisibleDimensions([2, 3, 4]);
 
-      obs = [dv.markers[0].position.x, dv.markers[0].position.y,
+      var obs = [dv.markers[0].position.x, dv.markers[0].position.y,
              dv.markers[0].position.z];
       var exp = [0.066647, -0.067711, 0.176070];
       deepEqual(obs, exp);
@@ -375,7 +454,7 @@ requirejs([
       UIState1.setProperty('view.usesPointCloud', false);
       var dv = new DecompositionView(this.multiModel, 'scatter', UIState1);
       dv.changeVisibleDimensions([2, 3, null]);
-      obs = [dv.markers[0].position.x,
+      var obs = [dv.markers[0].position.x,
       dv.markers[0].position.y,
       dv.markers[0].position.z];
       exp = [0.066647, -0.067711, 0];
@@ -436,7 +515,7 @@ requirejs([
       // 1.- The position themselves
       // 2.- The ranges i.e. positions still fall within the dimensionRanges.
       // 3.- The axis orientation vector
-      obs = dv.markers[0].position.toArray();
+      var obs = dv.markers[0].position.toArray();
       deepEqual(obs, expa, 'First marker position updated correctly');
 
       assert.ok(obs[1] <= dv.decomp.dimensionRanges.max[1],
@@ -472,7 +551,7 @@ requirejs([
       // 1.- The position themselves
       // 2.- The ranges i.e. positions still fall within the dimensionRanges.
       // 3.- The axis orientation vector
-      obs = dv.markers[0].position.toArray();
+      var obs = dv.markers[0].position.toArray();
       deepEqual(obs, expa, 'First marker position updated correctly');
       obs = dv.markers[1].position.toArray();
       deepEqual(obs, expb, 'Second marker position updated correctly');
@@ -526,7 +605,7 @@ requirejs([
       deepEqual(dv.axesOrientation, [1, 1, 1]);
 
       dv.changeVisibleDimensions([2, 3, 4]);
-      obs = dv.markers[0].position.toArray();
+      var obs = dv.markers[0].position.toArray();
       exp = [0.066647, -0.067711, 0.176070];
       deepEqual(obs, exp, 'First marker position updated correctly');
 

tests/javascript_tests/test_util.js

@@ -133,42 +133,42 @@ requirejs(['jquery', 'underscore', 'util'], function($, _, util) {
 
     /**
      *
-     * Test that strings like "Infinity" are sorted with words in
+     * Test that strings like 'Infinity' are sorted with words in
      * naturalSort's output, rather than with numbers. This test was adapted
      * from Empress.
      *
      */
-    test("Test that naturalSort doesn't treat Infinity / NaN as numbers", function () {
+    test("Test naturalSort doesn't treat Infinity/NaN as numbers", function() {
       var eles = [
-        "1",
-        "2",
-        "3",
-        "10",
-        "4",
-        "5",
-        "invalid",
-        "nan",
-        "NaN",
-        "Infinity",
-        "-Infinity",
-        " ",
-        "zzz",
+        '1',
+        '2',
+        '3',
+        '10',
+        '4',
+        '5',
+        'invalid',
+        'nan',
+        'NaN',
+        'Infinity',
+        '-Infinity',
+        ' ',
+        'zzz',
       ];
       res = naturalSort(eles);
       deepEqual(res, [
-        " ",
-        "-Infinity",
-        "Infinity",
-        "invalid",
-        "nan",
-        "NaN",
-        "zzz",
-        "1",
-        "2",
-        "3",
-        "4",
-        "5",
-        "10",
+        ' ',
+        '-Infinity',
+        'Infinity',
+        'invalid',
+        'nan',
+        'NaN',
+        'zzz',
+        '1',
+        '2',
+        '3',
+        '4',
+        '5',
+        '10',
       ]);
     });