Handle categorical colorschemes correctly; add tests for colorer.js; …

…fix a tiny bug (#159)

* TST: Add test-colorer.js skeleton #142 #137

* TST: Test that QIIME colorscheme matches Emperor

:100:

* BUG/ENH: Fix discrete colormaps (close #137)

This replaces Colorer.__colorer with Colorer.__colorArray, which
is what it sounds like on the tin (an array of hex color codes) :)

The downside is that this treats quant. color schemes the same
way as discrete color palettes: so these palettes are just looped,
rather than being scaled to fit the entire range of items.

I'm of the opinion that applying quant. color schemes to discrete
data is a bit useless, so I think the next step is adding some sort
of button / checkbox / etc. that lets the user select if they want
color setting to be done with the looping method (added here) or
the scaling method (as was done previously). Will need to think about
what provides the best UX (I think these defaults will be better
than before, at least).

* TST: test discrete colormap construction for #137

* BUG/TST: Fix color precision bug; add tst for RGB

255 instead of 256 for scaling. See new comment in colorer.js for
explanation.

realized that prettier wasn't being run on test js code, so just
pulled that trigger. will commit changes for remaining test-*.js
files shortly.

* TST: ... actually declare module for colorer tsts

WHOOPS didn't realize that keeping that line was important

* TST: test getColorHex()

annnd I think we're good here!

* DOC: finally, redo qzv

* MNT: Simplify getColorRGB() significantly

Turns out chroma(some color).gl() is a function! dang.

* TST: use chroma(color).hex() as expected

rather than assuming that chroma(color)'s default repr will be
as hex (I think that's a safe assumption, but best not to bake
too many assumptions into the tests...)

* MNT: Add util.js with naturalSort()

Per @ElDeveloper's suggestion in #159

* BUG: utils -> util in module name references

oops, that broke the js

Ok, TODOs from here are:

  -Use util.naturalSort() to sort categories, both for coloring AND
   for placement in the legend (or ideally, just sort once and reuse
   the results)

  -If a sequential / diverging colormap is selected, interpolate
   values analogously to what Emperor does. This will just be
   "ordinal" for now, so e.g. [1, 2, 3, 4] will result in the same
   colors assigned as [1, 100, 1000, 10000000] or any other 4-category
   field.

* TST: Port over util.naturalSort tests #159

Includes creating a new test-util.js module. (I imagine we'll probably
add more util functions and their tests to util.js + test-util.js
in the future.)

* BUG: use naturalSort() for all sample categories

This could be made more efficient (i.e. only doing the sorting op.
once and saving the result rather than doing it in two diff. places)
but just wanna get this working for now.

Weirdly enough the color stuff seems to be malfunctioning now? I don't
think this commit is the cause but I think something on this branch
got messed up... I'll check

* BUG: Fix funky WebGL coloring bug

See test comments for explanation. TLDR: WebGL (or maybe some
intermediate utility) misinterprets the opacity component in the
color array, which breaks things.

ACTUALLY: thinking about it now, I think this bug might actually be
due to Empress' code, which mixes up colors and coordinates into
a single array when drawing stuff. I bet that that's it -- the "1" is
probably interpreted as a coordinate, which is why things look so
weird.

* DOC: update comments re: cause of prev bug

* MNT: Add splitNumericValues() & port Emperor tests

* MNT: Move keepUniqueKeys() to util module

I assume this function could be useful for other contexts, and porting
it to this module should make testing it a bit easier (which will be
useful when refactoring it; #147).

I haven't actually added tests for this function yet, mostly because
I'm having a hard time following it / understanding what it's doing.

* ENH: Scale numeric vals for SEQ/DIV colormaps

Addresses @ElDeveloper's comments in #159.

The code for this is a bit clunky (a lot of logic should be moved
out of the Colorer constructor; the alerts happen twice when they
should only happen once; tests are broken), but from here on should
be ok

* MNT: Simplify Colorer obj a lot

Rather than creating multiple Colorers for the RGB and Hex stuff,
this just creates one Colorer and reuses it for both color formats.

This lets us completely do away with Empress._assignColor()! And
some other stuff, too.

* MNT: Split up Colorer constructor to sep funcs

also updated QZV, verified that it seems to work (still need
to update colorer tests tho)

* TST: Update discrete color palette test

* MNT/TST: Test color scaling; bump chroma to v2.1

Turns out that the Chroma.js version we were using here was ~4 years
out of date -- I noticed small discrepancies (like, on the order of
~0x01 precision for R/G/B channels of the hex colors) between our
Chroma.js and the Chroma.js docs when writing this test.

It looks like Chroma.js version 1.3.3 improved precision, which is
probably the (main?) cause of the observed discrepancies.

I confirmed that updating the version fixed the test errors.
This implies that we should bump the Chroma version for Emperor
/ other libraries using old versions of Chroma also.

* TST: add more color scaling tests

* TST: Finally, test the new Colorer.getMap funcs

* DOC/BUG: Document getMap funcs; minor class thing

See getMapHex()'s interior for some details on the change made in
this commit

* TST/MNT: Defer alert on scaling fail to post tests

This is really just kicking the can down the road, but we can do
that ad infinitum as needed I guess. (... or we can just bite the
bullet and inevitably spend like 10 hours messing with Puppeteer)

* DOC: update screenshot now that #137 fixed

* BUG: fix inconsistent legend order

Needed to slap on a call to naturalSort() in the legend code. It would
be best to instead just sort once and pass the sorted list through
the chain of code... that's a TODO. (Also testing would help make
me more confident about this :)

* MNT: Replace "ccm" scaling with "ccwm" scaling

So, things are now scaled ordinally. See comments of new function
in colorer.js for details.

* TST: Update colorer tests re: ordinal scaling chgs

* BUG: Update animation to work with new Color API

This seems to function as expected, but it would be good to check
that this didn't break things

* BUG/TST: Fix and test single-value color case

Forgot about this. should be good now.

* BUG: Put Infinities with words in naturalSort

* DOC: update demo QZV to use latest JS from this PR

empress/support_files/js/animator.js

@@ -139,9 +139,6 @@ define(["Colorer"], function (Colorer) {
         hide,
         lWidth
     ) {
-        // used in closure
-        var animator = this;
-
         this.gradientCol = gradient;
         this.gradientSteps = this.empress.getUniqueSampleValues(gradient);
         this.totalFrames = Object.keys(this.gradientSteps).length;
@@ -155,15 +152,9 @@ define(["Colorer"], function (Colorer) {
         var trajectories = this.empress.getUniqueSampleValues(trajectory);
 
         // Assign a color to each unique category
-        this.cm = {};
-        this.legendInfo = {};
-        var colorer = new Colorer(cm, 0, trajectories.length);
-
-        // assign each trajectory a color and store it in this.cm
-        trajectories.forEach(function (x, i) {
-            animator.cm[x] = { color: colorer.getColorRGB(i) };
-            animator.legendInfo[x] = { color: colorer.getColorHex(i) };
-        });
+        var colorer = new Colorer(cm, trajectories);
+        this.cm = colorer.getMapRGB();
+        this.legendInfo = colorer.getMapHex();
 
         this.hide = hide;
         this.lWidth = lWidth;

empress/support_files/js/colorer.js

@@ -1,56 +1,139 @@
-define(["chroma"], function (chroma) {
-    // class globals closure variables
-    var DISCRETE = "Discrete";
-    var SEQUENTIAL = "Sequential";
-    var DIVERGING = "Diverging";
-    var HEADER = "Header";
-
+define(["chroma", "underscore", "util"], function (chroma, _, util) {
     /**
      * @class Colorer
      *
      * Creates a color object that will map values to colors from a pre-defined
-     * color map. The color object uses draws from a range of values defined by
-     * [min, max] and assigns a color based on where the value falls within the
-     * range.
+     * color map.
      *
-     * @param{Object} color The color map to draw colors from.
-     * @param{Object} min The minimum value.
-     * @param{Object} max The maximum value.
+     * @param{String} color The color map to draw colors from.
+     *                      This should be an id in Colorer.__Colormaps.
+     * @param{Array} values The values in a metadata field for which colors
+     *                      will be generated.
      *
      * @return{Colorer}
      * constructs Colorer
      */
-    function Colorer(color, min, max) {
-        if (color === Colorer.__QIIME_COLOR) {
-            this.__colorer = chroma.scale(Colorer.__qiimeDiscrete);
+    function Colorer(color, values) {
+        // Remove duplicate values and sort the values sanely
+        this.sortedUniqueValues = util.naturalSort(_.uniq(values));
+
+        this.color = color;
+
+        // This object will describe a mapping of unique field values to colors
+        this.__valueToColor = {};
+
+        // Figure out what "type" of color map has been selected (should be one
+        // of discrete, sequential, or diverging)
+        this.selectedColorMap = _.find(Colorer.__Colormaps, function (cm) {
+            return cm.id === color;
+        });
+
+        // Based on the determined color map type, assign colors accordingly
+        if (this.selectedColorMap.type === Colorer.DISCRETE) {
+            this.assignDiscreteColors();
+        } else if (
+            this.selectedColorMap.type === Colorer.SEQUENTIAL ||
+            this.selectedColorMap.type === Colorer.DIVERGING
+        ) {
+            this.assignOrdinalScaledColors();
         } else {
-            this.__colorer = chroma.scale(color);
+            throw new Error("Invalid color map " + this.color + " specified");
         }
-        this.__colorer.domain([min, max]);
     }
 
     /**
-     * Returns an rgb array with values in the range of [0,1].
+     * Assigns colors from a discrete color palette (specified by this.color)
+     * for every value in this.sortedUniqueValues. This will populate
+     * this.__valueToColor with this information.
      *
-     * @param{Number} color A number in the range [min,max]
+     * This will "loop around" as needed in order to generate colors; for
+     * example, if the color palette has 10 colors and there are 15 elements in
+     * this.sortedUniqueValues, the last 5 of those 15 elements will be
+     * assigned the first 5 colors from the color palette.
+     */
+    Colorer.prototype.assignDiscreteColors = function () {
+        var palette;
+        if (this.color === Colorer.__QIIME_COLOR) {
+            palette = Colorer.__qiimeDiscrete;
+        } else {
+            palette = chroma.brewer[this.color];
+        }
+        for (var i = 0; i < this.sortedUniqueValues.length; i++) {
+            var modIndex = i % palette.length;
+            this.__valueToColor[this.sortedUniqueValues[i]] = palette[modIndex];
+        }
+    };
+
+    /**
+     * Assigns colors from a sequential or diverging color palette (specified
+     * by this.color) for every value in this.sortedUniqueValues. This will
+     * populate this.__valueToColor with this information.
      *
-     * @return{Object} An rgb array
+     * Note the "ordinal" in the function name. This does not take into account
+     * the actual magnitudes of numbers in the data -- all that matters is the
+     * order of the values (and thanks to our use of util.naturalSort() we know
+     * that values interpretable as numbers will get sorted correctly).
+     * So, as an example of that, the values [1, 2, 3, 100] will get assigned
+     * the same colors as [1, 2, 3, 4] or [a, b, 1, 2] or [a, b, c, d].
      */
-    Colorer.prototype.getColorRGB = function (color) {
-        return this.__colorer(color)
-            .rgb()
-            .map((x) => x / 256);
+    Colorer.prototype.assignOrdinalScaledColors = function () {
+        if (this.sortedUniqueValues.length === 1) {
+            // If there's only 1 unique value, set its color as the first in
+            // the color map. This matches the behavior of Emperor.
+            var onlyVal = this.sortedUniqueValues[0];
+            this.__valueToColor[onlyVal] = chroma.brewer[this.color][0];
+        } else {
+            // ... Otherwise, do normal interpolation -- the first value gets
+            // the "first" color in the colormap, the last value gets the
+            // "last" color in the colormap, and things in between are
+            // interpolated. Chroma takes care of all of the hard work.
+            var interpolator = chroma
+                .scale(chroma.brewer[this.color])
+                .domain([0, this.sortedUniqueValues.length - 1]);
+
+            for (var i = 0; i < this.sortedUniqueValues.length; i++) {
+                var val = this.sortedUniqueValues[i];
+                this.__valueToColor[val] = interpolator(i);
+            }
+        }
     };
 
     /**
-     * Returns an rgb hex string.
+     * Returns a mapping of unique field values to their corresponding colors,
+     * where each color is in RGB array format.
      *
-     * @param{Number} color A number in the range [min,max]
+     * @return{Object} rgbMap An object mapping each item in
+     *                 this.sortedUniqueValues to its assigned color. Each
+     *                 color is represented by an array of [R, G, B], where R,
+     *                 G, B are all floats scaled to within the range [0, 1].
+     */
+    Colorer.prototype.getMapRGB = function () {
+        return _.mapObject(this.__valueToColor, function (color) {
+            // chroma(color).gl() returns an array with four components (RGBA
+            // instead of RGB). The slice() here strips off the final
+            // (transparency) element, which causes problems with Empress'
+            // drawing code
+            return chroma(color).gl().slice(0, 3);
+        });
+    };
+
+    /**
+     * Returns a mapping of unique field values to their corresponding colors,
+     * where each color is in hex format.
      *
-     * @return{Object} An rgb hex string
+     * @return{Object} hexMap An object mapping each item in
+     *                 this.sortedUniqueValues to its assigned color. Each
+     *                 color is represented by a hex string like "#ff0000".
      */
-    Colorer.prototype.getColorHex = function (color) {
-        return this.__colorer(color).hex();
+    Colorer.prototype.getMapHex = function () {
+        // Technically we already store colors in hex format, so we could just
+        // return this.__valueToColor directly. However, JS is "call by
+        // sharing," so this would allow a user to overwrite the values in the
+        // returned object and thus mess with the Colorer internals. Hence why
+        // we return a shallow copy of this.__valueToColor instead (the object
+        // doesn't include other objects / arrays within it, so this should be
+        // safe). See https://stackoverflow.com/a/5314911/10730311 for details.
+        return _.clone(this.__valueToColor);
     };
 
     /**
@@ -74,6 +157,11 @@ define(["chroma"], function (chroma) {
         }
     };
 
+    Colorer.DISCRETE = "Discrete";
+    Colorer.SEQUENTIAL = "Sequential";
+    Colorer.DIVERGING = "Diverging";
+    Colorer.HEADER = "Header";
+
     // taken from the qiime/colors.py module; a total of 24 colors
     /** @private */
     Colorer.__QIIME_COLOR = "discrete-coloring-qiime";
@@ -104,57 +192,61 @@ define(["chroma"], function (chroma) {
         "#008080",
     ];
 
+    // This is also the default "nanColor" for Emperor. (We could make this
+    // configurable if desired.)
+    Colorer.NANCOLOR = "#64655d";
+
     // Used to create color select option and chroma.brewer
     //Modified from:
     //https://github.com/biocore/emperor/blob/
     //     027aa16f1dcf9536cd2dd9c9800ece5fc359ecbc/emperor/
     //     support_files/js/color-view-controller.js#L573-L613
     Colorer.__Colormaps = [
-        { name: "-- Discrete --", type: HEADER },
+        { name: "-- Discrete --", type: Colorer.HEADER },
         {
             id: "discrete-coloring-qiime",
             name: "Classic QIIME Colors",
-            type: DISCRETE,
+            type: Colorer.DISCRETE,
         },
-        { id: "Paired", name: "Paired", type: DISCRETE },
-        { id: "Accent", name: "Accent", type: DISCRETE },
-        { id: "Dark2", name: "Dark", type: DISCRETE },
-        { id: "Set1", name: "Set1", type: DISCRETE },
-        { id: "Set2", name: "Set2", type: DISCRETE },
-        { id: "Set3", name: "Set3", type: DISCRETE },
-        { id: "Pastel1", name: "Pastel1", type: DISCRETE },
-        { id: "Pastel2", name: "Pastel2", type: DISCRETE },
-
-        { name: "-- Sequential --", type: HEADER },
-        { id: "Viridis", name: "Viridis", type: SEQUENTIAL },
-        { id: "Reds", name: "Reds", type: SEQUENTIAL },
-        { id: "RdPu", name: "Red-Purple", type: SEQUENTIAL },
-        { id: "Oranges", name: "Oranges", type: SEQUENTIAL },
-        { id: "OrRd", name: "Orange-Red", type: SEQUENTIAL },
-        { id: "YlOrBr", name: "Yellow-Orange-Brown", type: SEQUENTIAL },
-        { id: "YlOrRd", name: "Yellow-Orange-Red", type: SEQUENTIAL },
-        { id: "YlGn", name: "Yellow-Green", type: SEQUENTIAL },
-        { id: "YlGnBu", name: "Yellow-Green-Blue", type: SEQUENTIAL },
-        { id: "Greens", name: "Greens", type: SEQUENTIAL },
-        { id: "GnBu", name: "Green-Blue", type: SEQUENTIAL },
-        { id: "Blues", name: "Blues", type: SEQUENTIAL },
-        { id: "BuGn", name: "Blue-Green", type: SEQUENTIAL },
-        { id: "BuPu", name: "Blue-Purple", type: SEQUENTIAL },
-        { id: "Purples", name: "Purples", type: SEQUENTIAL },
-        { id: "PuRd", name: "Purple-Red", type: SEQUENTIAL },
-        { id: "PuBuGn", name: "Purple-Blue-Green", type: SEQUENTIAL },
-        { id: "Greys", name: "Greys", type: SEQUENTIAL },
-
-        { name: "-- Diverging --", type: HEADER },
-        { id: "Spectral", name: "Spectral", type: DIVERGING },
-        { id: "RdBu", name: "Red-Blue", type: DIVERGING },
-        { id: "RdYlGn", name: "Red-Yellow-Green", type: DIVERGING },
-        { id: "RdYlBu", name: "Red-Yellow-Blue", type: DIVERGING },
-        { id: "RdGy", name: "Red-Grey", type: DIVERGING },
-        { id: "PiYG", name: "Pink-Yellow-Green", type: DIVERGING },
-        { id: "BrBG", name: "Brown-Blue-Green", type: DIVERGING },
-        { id: "PuOr", name: "Purple-Orange", type: DIVERGING },
-        { id: "PRGn", name: "Purple-Green", type: DIVERGING },
+        { id: "Paired", name: "Paired", type: Colorer.DISCRETE },
+        { id: "Accent", name: "Accent", type: Colorer.DISCRETE },
+        { id: "Dark2", name: "Dark", type: Colorer.DISCRETE },
+        { id: "Set1", name: "Set1", type: Colorer.DISCRETE },
+        { id: "Set2", name: "Set2", type: Colorer.DISCRETE },
+        { id: "Set3", name: "Set3", type: Colorer.DISCRETE },
+        { id: "Pastel1", name: "Pastel1", type: Colorer.DISCRETE },
+        { id: "Pastel2", name: "Pastel2", type: Colorer.DISCRETE },
+
+        { name: "-- Sequential --", type: Colorer.HEADER },
+        { id: "Viridis", name: "Viridis", type: Colorer.SEQUENTIAL },
+        { id: "Reds", name: "Reds", type: Colorer.SEQUENTIAL },
+        { id: "RdPu", name: "Red-Purple", type: Colorer.SEQUENTIAL },
+        { id: "Oranges", name: "Oranges", type: Colorer.SEQUENTIAL },
+        { id: "OrRd", name: "Orange-Red", type: Colorer.SEQUENTIAL },
+        { id: "YlOrBr", name: "Yellow-Orange-Brown", type: Colorer.SEQUENTIAL },
+        { id: "YlOrRd", name: "Yellow-Orange-Red", type: Colorer.SEQUENTIAL },
+        { id: "YlGn", name: "Yellow-Green", type: Colorer.SEQUENTIAL },
+        { id: "YlGnBu", name: "Yellow-Green-Blue", type: Colorer.SEQUENTIAL },
+        { id: "Greens", name: "Greens", type: Colorer.SEQUENTIAL },
+        { id: "GnBu", name: "Green-Blue", type: Colorer.SEQUENTIAL },
+        { id: "Blues", name: "Blues", type: Colorer.SEQUENTIAL },
+        { id: "BuGn", name: "Blue-Green", type: Colorer.SEQUENTIAL },
+        { id: "BuPu", name: "Blue-Purple", type: Colorer.SEQUENTIAL },
+        { id: "Purples", name: "Purples", type: Colorer.SEQUENTIAL },
+        { id: "PuRd", name: "Purple-Red", type: Colorer.SEQUENTIAL },
+        { id: "PuBuGn", name: "Purple-Blue-Green", type: Colorer.SEQUENTIAL },
+        { id: "Greys", name: "Greys", type: Colorer.SEQUENTIAL },
+
+        { name: "-- Diverging --", type: Colorer.HEADER },
+        { id: "Spectral", name: "Spectral", type: Colorer.DIVERGING },
+        { id: "RdBu", name: "Red-Blue", type: Colorer.DIVERGING },
+        { id: "RdYlGn", name: "Red-Yellow-Green", type: Colorer.DIVERGING },
+        { id: "RdYlBu", name: "Red-Yellow-Blue", type: Colorer.DIVERGING },
+        { id: "RdGy", name: "Red-Grey", type: Colorer.DIVERGING },
+        { id: "PiYG", name: "Pink-Yellow-Green", type: Colorer.DIVERGING },
+        { id: "BrBG", name: "Brown-Blue-Green", type: Colorer.DIVERGING },
+        { id: "PuOr", name: "Purple-Orange", type: Colorer.DIVERGING },
+        { id: "PRGn", name: "Purple-Green", type: Colorer.DIVERGING },
     ];
     return Colorer;
 });