Merge pull request #1519 from plotly/bar-non-numeric-bug

Bar with non-numeric (x,y) items bug fix

Author: etpinard

src/traces/bar/calc.js

@@ -49,18 +49,11 @@ module.exports = function calc(gd, trace) {
 
     // create the "calculated data" to plot
     var serieslen = Math.min(pos.length, size.length),
-        cd = [];
+        cd = new Array(serieslen);
 
-    // set position
+    // set position and size
     for(i = 0; i < serieslen; i++) {
-
-        // add bars with non-numeric sizes to calcdata
-        // so that ensure that traces with gaps are
-        // plotted in the correct order
-
-        if(isNumeric(pos[i])) {
-            cd.push({p: pos[i]});
-        }
+        cd[i] = { p: pos[i], s: size[i] };
     }
 
     // set base
@@ -84,13 +77,6 @@ module.exports = function calc(gd, trace) {
         }
     }
 
-    // set size
-    for(i = 0; i < cd.length; i++) {
-        if(isNumeric(size[i])) {
-            cd[i].s = size[i];
-        }
-    }
-
     // auto-z and autocolorscale if applicable
     if(hasColorscale(trace, 'marker')) {
         colorscaleCalc(trace, trace.marker.color, 'marker', 'c');

src/traces/bar/set_positions.js

@@ -10,6 +10,7 @@
 'use strict';
 
 var isNumeric = require('fast-isnumeric');
+var BADNUM = require('../../constants/numerical').BADNUM;
 
 var Registry = require('../../registry');
 var Axes = require('../../plots/cartesian/axes');
@@ -37,8 +38,7 @@ module.exports = function setPositions(gd, plotinfo) {
             fullTrace.visible === true &&
             Registry.traceIs(fullTrace, 'bar') &&
             fullTrace.xaxis === xa._id &&
-            fullTrace.yaxis === ya._id &&
-            !calcTraces[i][0].placeholder
+            fullTrace.yaxis === ya._id
         ) {
             if(fullTrace.orientation === 'h') {
                 calcTracesHorizontal.push(calcTraces[i]);
@@ -188,7 +188,7 @@ function setGroupPositionsInStackOrRelativeMode(gd, pa, sa, calcTraces) {
         for(var j = 0; j < calcTrace.length; j++) {
             var bar = calcTrace[j];
 
-            if(!isNumeric(bar.s)) continue;
+            if(bar.s === BADNUM) continue;
 
             var isOutmostBar = ((bar.b + bar.s) === sieve.get(bar.p, bar.s));
             if(isOutmostBar) bar._outmost = true;
@@ -395,6 +395,8 @@ function setBarCenter(gd, pa, sieve) {
             calcBar[pLetter] = calcBar.p +
                 ((poffsetIsArray) ? poffset[j] : poffset) +
                 ((barwidthIsArray) ? barwidth[j] : barwidth) / 2;
+
+
         }
     }
 }
@@ -502,7 +504,7 @@ function stackBars(gd, sa, sieve) {
         for(j = 0; j < trace.length; j++) {
             bar = trace[j];
 
-            if(!isNumeric(bar.s)) continue;
+            if(bar.s === BADNUM) continue;
 
             // stack current bar and get previous sum
             var barBase = sieve.put(bar.p, bar.b + bar.s),
@@ -533,7 +535,7 @@ function sieveBars(gd, sa, sieve) {
         for(var j = 0; j < trace.length; j++) {
             var bar = trace[j];
 
-            if(isNumeric(bar.s)) sieve.put(bar.p, bar.b + bar.s);
+            if(bar.s !== BADNUM) sieve.put(bar.p, bar.b + bar.s);
         }
     }
 }
@@ -569,7 +571,7 @@ function normalizeBars(gd, sa, sieve) {
         for(var j = 0; j < trace.length; j++) {
             var bar = trace[j];
 
-            if(!isNumeric(bar.s)) continue;
+            if(bar.s === BADNUM) continue;
 
             var scale = Math.abs(sTop / sieve.get(bar.p, bar.s));
             bar.b *= scale;

src/traces/bar/sieve.js

@@ -11,6 +11,7 @@
 module.exports = Sieve;
 
 var Lib = require('../../lib');
+var BADNUM = require('../../constants/numerical').BADNUM;
 
 /**
  * Helper class to sieve data from traces into bins
@@ -34,7 +35,7 @@ function Sieve(traces, separateNegativeValues, dontMergeOverlappingData) {
         var trace = traces[i];
         for(var j = 0; j < trace.length; j++) {
             var bar = trace[j];
-            positions.push(bar.p);
+            if(bar.p !== BADNUM) positions.push(bar.p);
         }
     }
     this.positions = positions;

test/jasmine/assets/custom_matchers.js

@@ -143,7 +143,10 @@ function isClose(actual, expected, precision) {
         return Math.abs(actual - expected) < precision;
     }
 
-    return actual === expected;
+    return (
+        actual === expected ||
+        (isNaN(actual) && isNaN(expected))
+    );
 }
 
 function coercePosition(precision) {

test/jasmine/tests/bar_test.js

@@ -283,6 +283,39 @@ describe('Bar.calc', function() {
         var cd = gd.calcdata;
         assertPointField(cd, 'b', [[0, 1, 2], [0, 1, 0], [0, 0]]);
     });
+
+    it('should not exclude items with non-numeric x/y from calcdata', function() {
+        var gd = mockBarPlot([{
+            x: [5, NaN, 15, 20, null, 21],
+            y: [20, NaN, 23, 25, null, 26]
+        }]);
+
+        var cd = gd.calcdata;
+        assertPointField(cd, 'x', [[5, NaN, 15, 20, NaN, 21]]);
+        assertPointField(cd, 'y', [[20, NaN, 23, 25, NaN, 26]]);
+    });
+
+    it('should not exclude items with non-numeric y from calcdata (to plots gaps correctly)', function() {
+        var gd = mockBarPlot([{
+            x: ['a', 'b', 'c', 'd'],
+            y: [1, null, 'nonsense', 15]
+        }]);
+
+        var cd = gd.calcdata;
+        assertPointField(cd, 'x', [[0, 1, 2, 3]]);
+        assertPointField(cd, 'y', [[1, NaN, NaN, 15]]);
+    });
+
+    it('should not exclude items with non-numeric x from calcdata (to plots gaps correctly)', function() {
+        var gd = mockBarPlot([{
+            x: [1, null, 'nonsense', 15],
+            y: [1, 2, 10, 30]
+        }]);
+
+        var cd = gd.calcdata;
+        assertPointField(cd, 'x', [[1, NaN, NaN, 15]]);
+        assertPointField(cd, 'y', [[1, 2, 10, 30]]);
+    });
 });
 
 describe('Bar.setPositions', function() {
@@ -681,23 +714,6 @@ describe('Bar.setPositions', function() {
         expect(Axes.getAutoRange(ya)).toBeCloseToArray([-1.11, 1.11], undefined, '(ya.range)');
     });
 
-    it('should skip placeholder trace in position computations', function() {
-        var gd = mockBarPlot([{
-            x: [1, 2, 3],
-            y: [2, 1, 2]
-        }, {
-            x: [null],
-            y: [null]
-        }]);
-
-        expect(gd.calcdata[0][0].t.barwidth).toEqual(0.8);
-
-        expect(gd.calcdata[1][0].x).toBe(false);
-        expect(gd.calcdata[1][0].y).toBe(false);
-        expect(gd.calcdata[1][0].placeholder).toBe(true);
-        expect(gd.calcdata[1][0].t.barwidth).toBeUndefined();
-    });
-
     it('works with log axes (grouped bars)', function() {
         var gd = mockBarPlot([
             {y: [1, 10, 1e10, -1]},
@@ -1285,7 +1301,7 @@ function mockBarPlot(dataWithoutTraceType, layout) {
 
     var gd = {
         data: dataWithTraceType,
-        layout: layout,
+        layout: layout || {},
         calcdata: []
     };