From 3b7faf75d4339da03939f91d696b061883762e3a Mon Sep 17 00:00:00 2001 From: alexcjohnson Date: Mon, 12 Feb 2018 15:41:04 -0500 Subject: [PATCH] fix errorbars for Plotly.react and for uneven data arrays --- src/components/errorbars/compute_error.js | 16 +++++-- src/components/errorbars/defaults.js | 6 +-- src/components/errorbars/plot.js | 8 ++-- test/image/mocks/basic_error_bar.json | 5 ++ test/jasmine/tests/errorbars_test.js | 56 +++++++++++++++++++++-- test/jasmine/tests/plot_api_test.js | 1 + 6 files changed, 77 insertions(+), 15 deletions(-) diff --git a/src/components/errorbars/compute_error.js b/src/components/errorbars/compute_error.js index a7b6b2ad2a9..87512817aed 100644 --- a/src/components/errorbars/compute_error.js +++ b/src/components/errorbars/compute_error.js @@ -30,18 +30,26 @@ module.exports = function makeComputeError(opts) { symmetric = opts.symmetric; if(type === 'data') { - var array = opts.array, - arrayminus = opts.arrayminus; + var array = opts.array || []; - if(symmetric || arrayminus === undefined) { + if(symmetric) { return function computeError(dataPt, index) { var val = +(array[index]); return [val, val]; }; } else { + var arrayminus = opts.arrayminus || []; return function computeError(dataPt, index) { - return [+arrayminus[index], +array[index]]; + var val = +array[index]; + var valMinus = +arrayminus[index]; + // in case one is present and the other is missing, fill in 0 + // so we still see the present one. Mostly useful during manual + // data entry. + if(!isNaN(val) || !isNaN(valMinus)) { + return [valMinus || 0, val || 0]; + } + return [NaN, NaN]; }; } } diff --git a/src/components/errorbars/defaults.js b/src/components/errorbars/defaults.js index 500b3af2520..0eb5a2f75b5 100644 --- a/src/components/errorbars/defaults.js +++ b/src/components/errorbars/defaults.js @@ -44,12 +44,10 @@ module.exports = function(traceIn, traceOut, defaultColor, opts) { } if(type === 'data') { - var array = coerce('array'); - if(!array) containerOut.array = []; + coerce('array'); coerce('traceref'); if(!symmetric) { - var arrayminus = coerce('arrayminus'); - if(!arrayminus) containerOut.arrayminus = []; + coerce('arrayminus'); coerce('tracerefminus'); } } diff --git a/src/components/errorbars/plot.js b/src/components/errorbars/plot.js index 4e540263b68..d03187cfcd2 100644 --- a/src/components/errorbars/plot.js +++ b/src/components/errorbars/plot.js @@ -76,6 +76,7 @@ module.exports = function plot(traces, plotinfo, transitionOpts) { var path; + var yerror = errorbar.select('path.yerror'); if(yObj.visible && isNumeric(coords.x) && isNumeric(coords.yh) && isNumeric(coords.ys)) { @@ -88,8 +89,6 @@ module.exports = function plot(traces, plotinfo, transitionOpts) { if(!coords.noYS) path += 'm-' + yw + ',0h' + (2 * yw); // shoe - var yerror = errorbar.select('path.yerror'); - isNew = !yerror.size(); if(isNew) { @@ -105,7 +104,9 @@ module.exports = function plot(traces, plotinfo, transitionOpts) { yerror.attr('d', path); } + else yerror.remove(); + var xerror = errorbar.select('path.xerror'); if(xObj.visible && isNumeric(coords.y) && isNumeric(coords.xh) && isNumeric(coords.xs)) { @@ -117,8 +118,6 @@ module.exports = function plot(traces, plotinfo, transitionOpts) { if(!coords.noXS) path += 'm0,-' + xw + 'v' + (2 * xw); // shoe - var xerror = errorbar.select('path.xerror'); - isNew = !xerror.size(); if(isNew) { @@ -134,6 +133,7 @@ module.exports = function plot(traces, plotinfo, transitionOpts) { xerror.attr('d', path); } + else xerror.remove(); }); }); }; diff --git a/test/image/mocks/basic_error_bar.json b/test/image/mocks/basic_error_bar.json index 76b8749532c..5b21661dfe4 100644 --- a/test/image/mocks/basic_error_bar.json +++ b/test/image/mocks/basic_error_bar.json @@ -20,6 +20,11 @@ ], "visible": true }, + "error_x": { + "type": "data", + "symmetric": false, + "visible": true + }, "type": "scatter" } ] diff --git a/test/jasmine/tests/errorbars_test.js b/test/jasmine/tests/errorbars_test.js index 3f6f9bd49bf..6cf7a1ea900 100644 --- a/test/jasmine/tests/errorbars_test.js +++ b/test/jasmine/tests/errorbars_test.js @@ -15,15 +15,29 @@ describe('errorbar plotting', function() { afterEach(destroyGraphDiv); + function countBars(xCount, yCount) { + expect(d3.select(gd).selectAll('.xerror').size()).toBe(xCount); + expect(d3.select(gd).selectAll('.yerror').size()).toBe(yCount); + } + + function checkCalcdata(cdTrace, errorBarData) { + cdTrace.forEach(function(di, i) { + var ebi = errorBarData[i] || {}; + expect(di.xh).toBe(ebi.xh); + expect(di.xs).toBe(ebi.xs); + expect(di.yh).toBe(ebi.yh); + expect(di.ys).toBe(ebi.ys); + }); + } + it('should autorange to the visible bars and remove invisible bars', function(done) { - function check(xrange, yrange, xcount, ycount) { + function check(xrange, yrange, xCount, yCount) { var xa = gd._fullLayout.xaxis; var ya = gd._fullLayout.yaxis; expect(xa.range).toBeCloseToArray(xrange, 3); expect(ya.range).toBeCloseToArray(yrange, 3); - expect(d3.selectAll('.xerror').size()).toBe(xcount); - expect(d3.selectAll('.yerror').size()).toBe(ycount); + countBars(xCount, yCount); } Plotly.newPlot(gd, [{ y: [1, 2, 3], @@ -50,4 +64,40 @@ describe('errorbar plotting', function() { .catch(fail) .then(done); }); + + it('shows half errorbars and removes individual bars that disappear', function(done) { + Plotly.newPlot(gd, [{ + x: [0, 10, 20], + y: [30, 40, 50], + error_x: {type: 'data', array: [2, 3], visible: true, symmetric: false}, + error_y: {type: 'data', arrayminus: [4], visible: true, symmetric: false} + }]) + .then(function() { + countBars(2, 1); + checkCalcdata(gd.calcdata[0], [ + {xs: 0, xh: 2, ys: 26, yh: 30}, + {xs: 10, xh: 13} + ]); + + Plotly.restyle(gd, {'error_x.array': [[1]], 'error_y.arrayminus': [[5, 6]]}); + }) + .then(function() { + countBars(1, 2); + checkCalcdata(gd.calcdata[0], [ + {xs: 0, xh: 1, ys: 25, yh: 30}, + {ys: 34, yh: 40} + ]); + + Plotly.restyle(gd, {'error_x.array': [[7, 8]], 'error_y.arrayminus': [[9]]}); + }) + .then(function() { + countBars(2, 1); + checkCalcdata(gd.calcdata[0], [ + {xs: 0, xh: 7, ys: 21, yh: 30}, + {xs: 10, xh: 18} + ]); + }) + .catch(fail) + .then(done); + }); }); diff --git a/test/jasmine/tests/plot_api_test.js b/test/jasmine/tests/plot_api_test.js index b783f3d89ad..0d52ac76d28 100644 --- a/test/jasmine/tests/plot_api_test.js +++ b/test/jasmine/tests/plot_api_test.js @@ -2577,6 +2577,7 @@ describe('Test plot api', function() { ['axes_enumerated_ticks', require('@mocks/axes_enumerated_ticks.json')], ['axes_visible-false', require('@mocks/axes_visible-false.json')], ['bar_and_histogram', require('@mocks/bar_and_histogram.json')], + ['basic_error_bar', require('@mocks/basic_error_bar.json')], ['binding', require('@mocks/binding.json')], ['cheater_smooth', require('@mocks/cheater_smooth.json')], ['finance_style', require('@mocks/finance_style.json')],