Skip to content

fix errorbars for Plotly.react and for uneven data arrays #2360

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 12, 2018
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions src/components/errorbars/compute_error.js
Original file line number Diff line number Diff line change
@@ -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) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this (removing || arrayminus === undefined) isn't actually a change in behavior, since we were filling in [] at the supplyDefaults step anyway

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];
};
}
}
6 changes: 2 additions & 4 deletions src/components/errorbars/defaults.js
Original file line number Diff line number Diff line change
@@ -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');
}
}
8 changes: 4 additions & 4 deletions src/components/errorbars/plot.js
Original file line number Diff line number Diff line change
@@ -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();
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

another bug fixed here (and tested below) - if you deleted some data from your errorbar data arrays, the corresponding errorbar was not removed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch 🎣


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();
});
});
};
5 changes: 5 additions & 0 deletions test/image/mocks/basic_error_bar.json
Original file line number Diff line number Diff line change
@@ -20,6 +20,11 @@
],
"visible": true
},
"error_x": {
"type": "data",
"symmetric": false,
"visible": true
},
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added x errorbars with no data so that the image wouldn't change, but it would have failed before this fix in plot_api_test

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the comment 📚

"type": "scatter"
}
]
56 changes: 53 additions & 3 deletions test/jasmine/tests/errorbars_test.js
Original file line number Diff line number Diff line change
@@ -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);
});
});
1 change: 1 addition & 0 deletions test/jasmine/tests/plot_api_test.js
Original file line number Diff line number Diff line change
@@ -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')],