properly handle possibly negative z values (mask) and negative cmin/cmax (fall back to linear scale)

Author: bmaranville

src/components/colorbar/draw.js

@@ -957,6 +957,8 @@ function mockColorBarAxis(gd, opts, zrange) {
 
     var cbAxisIn = {
         type: opts.type || 'linear',
+        // Colorscale.calc guarantees zrange is strictly positive whenever
+        // opts.type is still 'log' by the time we get here
         range: opts.type === 'log' ?
             [Math.log10(zrange[0]), Math.log10(zrange[1])] : zrange,
         tickmode: opts.tickmode,

src/components/colorscale/calc.js

@@ -5,9 +5,24 @@ var isNumeric = require('fast-isnumeric');
 var Lib = require('../../lib');
 var extractOpts = require('./helpers').extractOpts;
 
+// mirrors Lib.aggNums's own recursion into nested (eg heatmap z) arrays, but
+// masks out non-positive values first - matching how cartesian log axes
+// exclude non-positive data from autorange (see findExtremes in
+// plots/cartesian/autorange.js)
+function maskNonPositive(vals) {
+    if(Lib.isArrayOrTypedArray(vals[0])) return vals.map(maskNonPositive);
+
+    var out = new Array(vals.length);
+    for(var i = 0; i < vals.length; i++) {
+        var v = vals[i];
+        out[i] = (isNumeric(v) && v > 0) ? v : undefined;
+    }
+    return out;
+}
+
 module.exports = function calc(gd, trace, opts) {
     var fullLayout = gd._fullLayout;
-    var vals = opts.vals;
+    var rawVals = opts.vals;
     var containerStr = opts.containerStr;
 
     var container = containerStr ?
@@ -20,6 +35,27 @@ module.exports = function calc(gd, trace, opts) {
     var max = cOpts.max;
     var mid = cOpts.mid;
 
+    var colorbar = cOpts.colorbar;
+    var isLog = !!(colorbar && colorbar.type === 'log');
+
+    if(isLog) {
+        // a log colorbar needs a strictly positive domain: an explicit
+        // non-positive cmin/cmax can never be logged, and if this trace's
+        // data is going to be scanned for an auto min/max but has no
+        // positive values at all, there's nothing to scale from. Either way,
+        // fall back to a linear colorbar instead of an undefined/negative range.
+        var fullyPinned = !auto && isNumeric(min) && isNumeric(max);
+        var hasExplicitNonPositive = (isNumeric(min) && min <= 0) || (isNumeric(max) && max <= 0);
+        var hasPositiveData = fullyPinned || isNumeric(Lib.aggNums(Math.max, null, maskNonPositive(rawVals)));
+
+        if(hasExplicitNonPositive || !hasPositiveData) {
+            isLog = false;
+            colorbar.type = 'linear';
+        }
+    }
+
+    var vals = isLog ? maskNonPositive(rawVals) : rawVals;
+
     var minVal = function() { return Lib.aggNums(Math.min, null, vals); };
     var maxVal = function() { return Lib.aggNums(Math.max, null, vals); };
 
@@ -43,7 +79,9 @@ module.exports = function calc(gd, trace, opts) {
         }
     }
 
-    if(auto && mid !== undefined) {
+    // a symmetric-around-cmid range is a linear-domain concept - skip it for
+    // log colorbars rather than risk pushing a positive min/max non-positive
+    if(auto && mid !== undefined && !isLog) {
         if(max - mid > mid - min) {
             min = mid - (max - mid);
         } else if(max - mid < mid - min) {

src/components/colorscale/helpers.js

@@ -118,12 +118,14 @@ function extractScale(cont) {
     var cmax = cOpts.max;
 
     // Check if a log type is defined on the colorbar
+    // (Colorscale.calc guarantees cmin/cmax are strictly positive whenever
+    // type is still 'log' by the time we get here - see maskNonPositive)
     var type = (cOpts.colorbar && cOpts.colorbar.type) || 'linear';
 
     // Convert domain boundaries to base-10 log if required
     if(type === 'log') {
-        if(cmin > 0) cmin = Math.log10(cmin);
-        if(cmax > 0) cmax = Math.log10(cmax);
+        cmin = Math.log10(cmin);
+        cmax = Math.log10(cmax);
     }
 
     var scl = cOpts.reversescale ?
@@ -217,30 +219,33 @@ function makeColorScaleFunc(specs, opts) {
 }
 
 function makeColorScaleFuncFromTrace(trace, opts) {
-    var baseColorFn = makeColorScaleFunc(extractScale(trace), opts);
     var cOpts = extractOpts(trace);
     var type = (cOpts.colorbar && cOpts.colorbar.type) || 'linear';
+    var baseColorFn = makeColorScaleFunc(extractScale(trace), opts);
 
-    // Wrap the base generator if we need to dynamically apply log transformations
-    if (type === 'log') {
-        var wrappedFunc = function(v) {
-            if (isNumeric(v)) {
-                if (v <= 0) return 'rgba(0,0,0,0)'; // Logarithm of zero/negative is undefined
-                return baseColorFn(Math.log10(v));
-            } else if (tinycolor(v).isValid()) {
-                return v;
-            } else {
-                return Color.defaultLine;
-            }
-        };
+    if(type !== 'log') return baseColorFn;
 
-        // Preserve native domain and range methods for the draw components
-        wrappedFunc.domain = baseColorFn.domain;
-        wrappedFunc.range = baseColorFn.range;
-        return wrappedFunc;
-    }
+    opts = opts || {};
+    var noNumericCheck = opts.noNumericCheck;
+    var returnArray = opts.returnArray;
+
+    // Wrap the base generator to dynamically apply log transformations,
+    // matching the array/string return shape makeColorScaleFunc would have used
+    var wrappedFunc = function(v) {
+        if(noNumericCheck || isNumeric(v)) {
+            // log of zero/negative is undefined - treat as out-of-range
+            if(v > 0) return baseColorFn(Math.log10(v));
+            return returnArray ? [0, 0, 0, 0] : 'rgba(0,0,0,0)';
+        } else if(tinycolor(v).isValid()) {
+            return v;
+        }
+        return Color.defaultLine;
+    };
 
-    return baseColorFn;
+    // Preserve native domain and range methods for the draw components
+    wrappedFunc.domain = baseColorFn.domain;
+    wrappedFunc.range = baseColorFn.range;
+    return wrappedFunc;
 }
 
 function colorArray2rbga(colorArray) {
@@ -261,4 +266,4 @@ module.exports = {
     flipScale: flipScale,
     makeColorScaleFunc: makeColorScaleFunc,
     makeColorScaleFuncFromTrace: makeColorScaleFuncFromTrace
-};
+};