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.

Sign up for GitHub

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

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

@alexcjohnson alexcjohnson Feb 12, 2018

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];
};
}
}
Expand Down
6 changes: 2 additions & 4 deletions src/components/errorbars/defaults.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}
}
Expand Down
8 changes: 4 additions & 4 deletions src/components/errorbars/plot.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand All @@ -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) {
Expand All @@ -105,7 +104,9 @@ module.exports = function plot(traces, plotinfo, transitionOpts) {

yerror.attr('d', path);
}
else yerror.remove();
Copy link
Collaborator Author

@alexcjohnson alexcjohnson Feb 12, 2018

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

@etpinard etpinard Feb 12, 2018

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)) {
Expand All @@ -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) {
Expand All @@ -134,6 +133,7 @@ module.exports = function plot(traces, plotinfo, transitionOpts) {

xerror.attr('d', path);
}
else xerror.remove();
});
});
};
Expand Down
5 changes: 5 additions & 0 deletions test/image/mocks/basic_error_bar.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@
],
"visible": true
},
"error_x": {
"type": "data",
"symmetric": false,
"visible": true
},
Copy link
Collaborator Author

@alexcjohnson alexcjohnson Feb 12, 2018

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

@etpinard etpinard Feb 12, 2018

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"
}
]
Expand Down
56 changes: 53 additions & 3 deletions test/jasmine/tests/errorbars_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand All @@ -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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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')],
Expand Down