Skip to content

Commit

Permalink
Some (mostly coloring-focused) bug fixes (#763)
Browse files Browse the repository at this point in the history
* BUG: Make naturalSort treat +/-Infinity as words

Modified the function and added a unit test verifying this.
Both modifications are from Empress' codebase, specifically in
the PR biocore/empress#159.

* MNT: Update Chroma.js to v2.1.0 (from v1.1.1) #762

As expected, this causes the JS tests to fail due to small precision
differences. Will need to update those.

* BUG: Fix interpolation for "ordinal" color scaling

Closes #760. Tests are broken, though, so will need to fix those
(along with tests broken from fixing #762).

* MNT: use "Viridis" instead of "viridis" as default

Both seem to be accepted by Chroma.js, but may as well be consistent
with what their docs use (also, the Emperor docstrings said "Viridis"
in spite of this not being followed).

* TST: Fix getInterpolatedColors() test for #760

* TST: More CVC test fixes for #760/#762

Also added an empress code attribution to getInterpolatedColors()

* TST: Partial work on fixing expected gradient SVGs

emphasis on "partial", would prefer to automate this

* TST: Update gradient SVGs to use new colors

Just did this by copying in the new output (finagled it to roughly
match the old expected output's formatting for purposes of making
the diff cleaner).

Now that tests pass, I'm gonna say that this officially closes #762 :)

* MNT: only call parseFloat() once in naturalSort()

Addresses comment from @wasade

* TST: Test naturalSort() with sci. notation numbers

Addresses @wasade's comment.

NOTE that #761 is still gonna mess things up, though. Will comment
accordingly in #763.
  • Loading branch information
fedarko authored May 16, 2020
1 parent 023b6ec commit 2eecf7a
Show file tree
Hide file tree
Showing 6 changed files with 343 additions and 247 deletions.
4 changes: 2 additions & 2 deletions LICENSE.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,10 @@ OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.


#### Chroma (v1.1.1)
#### Chroma (v2.1.0)
[chroma.js](https://github.com/gka/chroma.js) - JavaScript library for color conversions

Copyright (c) 2011-2013, Gregor Aisch
Copyright (c) 2011-2019, Gregor Aisch
All rights reserved.

Redistribution and use in source and binary forms, with or without
Expand Down
12 changes: 7 additions & 5 deletions emperor/support_files/js/color-view-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ define([
*
*/
ColorViewController.getScaledColors = function(values, map, nanColor) {
map = map || 'viridis';
map = map || 'Viridis';
nanColor = nanColor || '#64655d';
map = chroma.brewer[map];

Expand Down Expand Up @@ -454,21 +454,23 @@ define([
*
* @param {String[]} values Objects to generate a color for, usually a
* category in a given metadata column.
* @param {String} [map = 'Viridis'] name of the discrete color map to use.
* @param {String} [map = 'Viridis'] name of the color map to use.
*
* @return {Object} colors The object containing the hex colors keyed to
* each sample.
*
*/
ColorViewController.getInterpolatedColors = function(values, map) {
map = map || 'viridis';
map = map || 'Viridis';
map = chroma.brewer[map];

var total = values.length;
var interpolator = chroma.bezier([map[0], map[3], map[4], map[5], map[8]]);
// Logic here adapted from Colorer.assignOrdinalScaledColors() in Empress'
// codebase
var interpolator = chroma.scale(map).domain([0, values.length - 1]);
var colors = {};
for (var i = 0; i < values.length; i++) {
colors[values[i]] = interpolator(i / total).hex();
colors[values[i]] = interpolator(i).hex();
}
return colors;
};
Expand Down
4 changes: 3 additions & 1 deletion emperor/support_files/js/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@ define(['underscore'], function(_) {
var numericPart = [], alphaPart = [], result = [];

// separate the numeric and the alpha elements of the array
// Note that NaN and +/- Infinity are not considered numeric elements
for (var index = 0; index < list.length; index++) {
if (isNaN(parseFloat(list[index]))) {
var valAsFloat = parseFloat(list[index]);
if (isNaN(valAsFloat) || !isFinite(valAsFloat)) {
alphaPart.push(list[index]);
}
else {
Expand Down
89 changes: 57 additions & 32 deletions emperor/support_files/vendor/js/chroma.min.js

Large diffs are not rendered by default.

Loading

0 comments on commit 2eecf7a

Please sign in to comment.