Support circular layout barplots; show legends for barplots using len…

…gth-scaling (#357)

* ENH: draw circular layout fm barplots! (hacky tho)

* BUG/ENH: correctly set "maxX" in c.bps+use 2 rects

* ENH: make s.m. barplots work in circular layout

how is this actually not that bad what

* DOC: upd8 comment

* DOC: update README re #297

* DOC: document polar -> Cartesian stuff

* DOC/PERF: mark some places to speed up cbarplots

since this kinda uh crashes the EMP tree LMAO

* PERF: reduce unnecessary work in c.barplot drawing

* PERF: chop out more work in c.sm.b. drawing

* MNT: abstract some more c. barplot drawing code

* DOC: document new util funcs for circ barplots

* DOC: clean up inner/outer radius terminology

* Typo fixes

* MNT: max X -> max displacement in bp drawing code

a more elegant way of representing this for circ barplots

* DOC: document precomputed y/angle stuff

* STY: pret

* MNT: shorten nodeAngleInfo var to just angleInfo

* MNT: Catch bad-layout case in _getNodeAngleInfo()

* ENH: Update "barplot unavail" msg re circ barplots

I would still like to add some tests to the coordinate functions,
but I think this is sufficient to close #297.

* PERF: don't draw barplots by dflt (close #343)

* DOC: punctuation

* TST: Test _getNodeAngleInfo()

* TST: add early addCircularBarCoords tst

* TST: finish x-coord circ bar coord test

just gotta test y-coords, with a similar switch stmt

* STY: prettify test

* TST: finish _addCircularBarCoords() test

* TST/MNT: abstract redundant test code

also add approxEqual func which I FEEL LIKE I ALREADY ADDED
but w/e

* TST/DOC: ref toFixedIfy et al in js layout tsts

* MNT: rm check for unsupported layouts in bpdrawing

* DOC: update comments in drawBarplots() start

* TST: Add more context/detail to circ bar comp tst

* DOC: clarify comment

* PERF: don't compute halfAngleRange w rect barplots

Makes things a bit more efficient.

* STY: prettify test error msg

* MNT: abstract rect coord-adding to sep func

* TST: test _addRectangularBarCoords()

* ENH: add length barplot legends; close #354

(still gotta test and document the new code, ofc)

* MNT: Simplify+doc layer drawing code re lenlegends

* TST: Clarify test util funcs

* DOC/TST: refurbish testing utilities docs

* TST: unbreak assignBarplotLengths() tests

* DOC/TST: polish up and test assignBarplotLengths

esp the new functionality. but also i apparently wasn't testing
non-numeric and numeric combos! that's wack. so now that's tested.

* TST: unbreak circ layout js tests

(since nodes are now just referred to by postorder positions)

* DOC: document more length legend stuff

* TST: test Legend.addLengthKey()

* STY: fix improperly named variable - jshint thing

* DOC: fix outdated comment

thanks @ElDeveloper!

* MNT: Simplify maxD computation in drawBarplots()

Thanks @ElDeveloper!

* DOC: fix old node desc in _getNodeAngleInfo()

thanks @kwcantrell!

* DOC: max displacement (not just x) in fm/sm coords

Forgot to update the docs. Thanks @kwcantrell for pointing this out.

Also changed a small block of code to just compute thisLayerMaxD
once and not like 3 times (?? why was I doing that lol)

* MNT: Compute max displacement in getLayoutInfo()

So that this is only done once per layout.

Addresses comment from @kwcantrell.

* STY: add missing semicolon

* TST: restructure approxDeepEqual funcs to use math

... and not stringification :P

Default epsilon is 1e-5, but for a few tests I only bothered writing
things out to 4 decimal places, so for these I use 1e-4.

* TST/STY: clean up utilities for testing

* DOC: tidy up max displacement wording

* BUG: Fix error when searching for invalid name

* DOC: update getNodesWithName() docs

* TST: fix approxDeepEqualMulti docs

thanks @ElDeveloper

* DOC: Update maxDisplacement description

thanks @kwcantrell

* MNT: add .gitattributes file marking vendored code

.gitattributes

@@ -0,0 +1,2 @@
+empress/support_files/vendor/* linguist-vendored=true
+tests/vendor/* linguist-vendored=true

README.md

@@ -120,7 +120,7 @@ The plot is now updated so each branch is now colored by its Phylum level classi
 
 ### Exploring a feature’s closest common ancestors  
 
-So far, we’ve looked at our data using the default unrooted tree view. To visually locate these features’ closest common ancestors, it may be easier to switch to a different layout. From the main menu, click *Layout* then select *Circular* (or *Rectangular*). Our plot automatically switches to a rooted circular cladogram.   
+So far, we’ve looked at our data using the default unrooted tree view. To visually locate these features’ closest common ancestors, it may be easier to switch to a different layout. From the main menu, click *Layout* then select *Circular* (or *Rectangular*). Our plot automatically switches to a rooted layout.
 
 Now zoom into the longest branch of the top cluster and click on the closest external node that has a different Phylum classification (light blue).   
 
@@ -155,7 +155,9 @@ obvious. To quote "Inferring Phylogenies" (Felsenstein 2004), pages 573–574:
 
 #### Diving into barplots: categorical feature metadata
 
-Barplots in Empress are currently only compatible with the rectangular layout, but support for circular-layout barplots is planned. To use barplots, change the layout to *Rectangular* (using the *Layout* section of the main menu), and then open up the *Barplots* section of the main menu and check the `Draw Barplots?` checkbox. By default, a red bar of uniform length will be drawn for every tip in the tree:
+Barplots in Empress are compatible with either the rectangular or circular layouts. Here we'll use the rectangular layout, but feel free to follow along with the circular layout if you prefer!
+
+First off, change the layout to *Rectangular* (using the *Layout* section of the main menu), and then open up the *Barplots* section of the main menu and check the `Draw Barplots?` checkbox. Click the *Update* button that appears. By default, a red bar of uniform length will be drawn for every tip in the tree:
 
 ![empress barplots initial view](docs/moving-pictures/img/empress_barplots_1.png)
 

empress/support_files/css/empress.css

@@ -254,6 +254,10 @@ select:-moz-focusring {
     background-color: white;
 }
 
+.header-cell {
+    font-weight: bold;
+}
+
 .menu-table tr td button:only-child:hover {
     background-color: darkgray;
 }

empress/support_files/js/barplot-layer.js

@@ -59,7 +59,8 @@ define([
         this.num = num;
         this.uniqueNum = uniqueNum;
 
-        this.legend = null;
+        this.colorLegend = null;
+        this.lengthLegend = null;
 
         this.fmAvailable = this.fmCols.length > 0;
 
@@ -104,7 +105,8 @@ define([
         this.layerDiv = null;
         this.fmDiv = null;
         this.smDiv = null;
-        this.legendDiv = null;
+        this.colorLegendDiv = null;
+        this.lengthLegendDiv = null;
         this.initHTML();
     }
 
@@ -564,15 +566,23 @@ define([
     };
 
     /**
-     * Initializes a <div> for this barplot layer that'll contain a legend.
+     * Initializes a <div> for this barplot layer that'll contain legends.
      */
     BarplotLayer.prototype.initLegendDiv = function () {
-        this.legendDiv = document.createElement("div");
-        this.legendDiv.classList.add("hidden");
-        this.legendDiv.classList.add("legend");
-        this.legendDiv.classList.add("barplot-layer-legend");
-        this.legend = new Legend(this.legendDiv);
-        this.layerDiv.appendChild(this.legendDiv);
+        this.colorLegendDiv = document.createElement("div");
+        this.colorLegendDiv.classList.add("hidden");
+        this.colorLegendDiv.classList.add("legend");
+        this.colorLegendDiv.classList.add("barplot-layer-legend");
+        this.colorLegend = new Legend(this.colorLegendDiv);
+        this.layerDiv.appendChild(this.colorLegendDiv);
+
+        this.lengthLegendDiv = document.createElement("div");
+        this.lengthLegendDiv.classList.add("hidden");
+        this.lengthLegendDiv.classList.add("legend");
+        this.lengthLegendDiv.classList.add("barplot-layer-legend");
+        this.lengthLegend = new Legend(this.lengthLegendDiv);
+        this.layerDiv.appendChild(this.lengthLegendDiv);
+
         // TODO: if possible, making the legend text selectable (overriding
         // the unselectable-text class on the side panel) would be nice, so
         // users can do things like highlight and copy category names.
@@ -592,7 +602,7 @@ define([
      * @param {Colorer} colorer Instance of a Colorer object defining the
      *                          current color selection for this barplot.
      */
-    BarplotLayer.prototype.populateLegend = function (colorer) {
+    BarplotLayer.prototype.populateColorLegend = function (colorer) {
         var isFM = this.barplotType === "fm";
         var title;
         if (isFM) {
@@ -608,28 +618,70 @@ define([
             !this.colorByFMColorMapDiscrete
         ) {
             var gradInfo = colorer.getGradientSVG();
-            this.legend.addContinuousKey(title, gradInfo[0], gradInfo[1]);
+            this.colorLegend.addContinuousKey(title, gradInfo[0], gradInfo[1]);
         } else {
-            this.legend.addCategoricalKey(title, colorer.getMapHex());
+            this.colorLegend.addCategoricalKey(title, colorer.getMapHex());
         }
     };
 
     /**
-     * Clears this layer's legend.
+     * Clears this layer's color legend.
      *
      * This is used when no color encoding is used for this layer -- this can
      * happen when the layer is for feature metadata, but the "Color by..."
      * checkbox is unchecked.
      *
      * NOTE that this is called even if the legend is already "cleared" --
-     * either this or populateLegend() is called once for every layer every
-     * time the barplots are redrawn. It'd be possible to try to save the state
-     * of the legend to avoid re-clearing / populating it, but I really doubt
-     * that this will be a bottleneck (unless there are, like, 1000 barplot
-     * layers at once).
+     * either this or populateColorLegend() is called once for every layer
+     * every time the barplots are redrawn. It'd be possible to try to save the
+     * state of the legend to avoid re-clearing / populating it, but I really
+     * doubt that this will be a bottleneck (unless there are, like, 1000
+     * barplot layers at once).
+     */
+    BarplotLayer.prototype.clearColorLegend = function () {
+        this.colorLegend.clear();
+    };
+
+    /**
+     * Populates the legend with information about the current length scaling
+     * in use.
+     *
+     * The circumstances in which this function is called are similar to those
+     * of populateColorLegend().
+     *
+     * @param {Number} minVal Minimum numeric value of the field used for
+     *                        length scaling.
+     * @param {Number} maxVal Maximum numeric value of the field used for
+     *                        length scaling.
+     * @throws {Error} If the current barplotType is not "fm". (Length scaling
+     *                 isn't supported for sample metadata barplots yet.)
+     */
+    BarplotLayer.prototype.populateLengthLegend = function (minVal, maxVal) {
+        var title;
+        if (this.barplotType === "fm") {
+            title = this.scaleLengthByFMField;
+            this.lengthLegend.addLengthKey(title, minVal, maxVal);
+        } else {
+            throw new Error(
+                "Length encoding is not supported for sample metadata " +
+                    "barplots yet."
+            );
+        }
+    };
+
+    /**
+     * Clears this layer's length legend.
+     *
+     * This is used when no length scaling is used for this layer -- this will
+     * (currently) always be the case for a sample metadata barplot layer.
+     *
+     * The circumstances in which this function is called are similar to those
+     * of clearColorLegend() (so, for example, this is called for each layer
+     * every time the barplots are updated; it's an inefficiency, but probably
+     * not a large one).
      */
-    BarplotLayer.prototype.clearLegend = function () {
-        this.legend.clear();
+    BarplotLayer.prototype.clearLengthLegend = function () {
+        this.lengthLegend.clear();
     };
 
     /**

empress/support_files/js/barplot-panel-handler.js

@@ -80,7 +80,9 @@ define(["underscore", "BarplotLayer", "Colorer"], function (
                 scope.addOptions.classList.remove("hidden");
                 scope.updateButton.classList.remove("hidden");
                 scope.enabled = true;
-                scope.empress.drawBarplots(scope.layers);
+                // We don't immediately draw barplots: see
+                // https://github.com/biocore/empress/issues/343. The user has
+                // to click "Update" first.
             } else {
                 scope.layerContainer.classList.add("hidden");
                 scope.addOptions.classList.add("hidden");
@@ -199,7 +201,7 @@ define(["underscore", "BarplotLayer", "Colorer"], function (
     /**
      * Array containing the names of layouts compatible with barplots.
      */
-    BarplotPanel.SUPPORTED_LAYOUTS = ["Rectangular"];
+    BarplotPanel.SUPPORTED_LAYOUTS = ["Rectangular", "Circular"];
 
     return BarplotPanel;
 });

empress/support_files/js/bp-tree.js

@@ -714,11 +714,13 @@ define(["ByteArray", "underscore"], function (ByteArray, _) {
     };
 
     /**
-     * Returns all node with a given name. Once a name has been searched for,
+     * Returns all nodes with a given name. Once a name has been searched for,
      * the returned object is cached in this._nameToNodes.
      *
      * @param {String} name The name of node(s)
-     * @return {Array} An array of postorder position of nodes with a given name
+     * @return {Array} An array of postorder positions of nodes with a given
+     *                 name. If no nodes have the specified name, this will be
+     *                 an empty array.
      */
     BPTree.prototype.getNodesWithName = function (name) {
         if (name in this._nameToNodes) {

empress/support_files/js/canvas-events.js

@@ -405,7 +405,7 @@ define(["glMatrix", "SelectedNodeMenu"], function (gl, SelectedNodeMenu) {
             // like the user searching for this name). Therefore, if there are
             // multiple nodes with this same name, things will be ambiguous.
             var nodeKeys = this.empress._tree.getNodesWithName(nodeName);
-            if (nodeKeys !== undefined) {
+            if (nodeKeys.length > 0) {
                 // At least one node with this name exists
                 openMenu(nodeKeys);
             } else {