Skip to content

Fix stack with gaps bar height bug #716

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 6 commits into from
Jul 6, 2016
Merged

Fix stack with gaps bar height bug #716

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b7f7944
bar: skip over bars with non-numeric size in setPositions
etpinard Jul 5, 2016
fa8aa5c
test: add stack with gaps mock
etpinard Jul 5, 2016
b9b1145
test-assets: add msg-extra argument to is-close array matchers
etpinard Jul 5, 2016
a2ad253
test-assets: don't try to compute dist between non-numeric elements
etpinard Jul 5, 2016
4cc0066
lint: flatten bar test suite
etpinard Jul 5, 2016
ef0c67f
test: add bar calc / setPositions tests
etpinard Jul 5, 2016
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
6 changes: 6 additions & 0 deletions src/traces/bar/calc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,13 @@ module.exports = function calc(gd, trace) {
// create the "calculated data" to plot
var serieslen = Math.min(pos.length, size.length),
cd = [];

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], s: size[i], b: 0});
}
Expand Down
5 changes: 5 additions & 0 deletions src/traces/bar/set_positions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,11 @@ module.exports = function setPositions(gd, plotinfo) {
for(i = 0; i < bl.length; i++) { // trace index
ti = gd.calcdata[bl[i]];
for(j = 0; j < ti.length; j++) {

// skip over bars with no size,
// so that we don't try to stack them
if(!isNumeric(ti[j].s)) continue;

sv = Math.round(ti[j].p / sumround);
// store the negative sum value for p at the same key, with sign flipped
if(relative && ti[j].s < 0) sv = -sv;
Expand Down
Binary file added test/image/baselines/bar_stack-with-gaps.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
175 changes: 175 additions & 0 deletions test/image/mocks/bar_stack-with-gaps.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,175 @@
{
"data": [
{
"x": [
"A",
"B",
"C",
"D",
"E",
"F",
"G"
],
"y": [
null,
null,
null,
null,
7,
null,
6
],
"name": "AA",
"type": "bar"
},
{
"x": [
"A",
"B",
"C",
"D",
"E",
"F",
"G"
],
"y": [
8,
null,
null,
null,
null,
null
],
"name": "BB",
"type": "bar"
},
{
"x": [
"A",
"B",
"C",
"D",
"E",
"F",
"G"
],
"y": [
null,
null,
null,
1,
3,
null
],
"name": "CC",
"type": "bar"
},
{
"x": [
"A",
"B",
"C",
"D",
"E",
"F",
"G"
],
"y": [
null,
4,
4,
null,
null,
null
],
"name": "DD",
"type": "bar"
},
{
"x": [
"A",
"B",
"C",
"D",
"E",
"F",
"G"
],
"y": [
null,
null,
null,
8,
null,
null,
3
],
"name": "EE",
"type": "bar"
},
{
"x": [
"A",
"B",
"C",
"D",
"E",
"F",
"G"
],
"y": [
null,
null,
null,
2,
null,
null
],
"name": "FF",
"type": "bar"
},
{
"x": [
"A",
"B",
"C",
"D",
"E",
"F",
"G"
],
"y": [
null,
null,
null,
null,
null,
1
],
"name": "GG",
"type": "bar"
}
],
"layout": {
"xaxis": {
"type": "category",
"range": [
-0.5,
6.5
],
"autorange": true
},
"barmode": "stack",
"yaxis": {
"type": "linear",
"range": [
0,
11.578947368421053
],
"autorange": true
},
"height": 450,
"width": 1100,
"autosize": true
}
}
32 changes: 24 additions & 8 deletions test/jasmine/assets/custom_matchers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,23 +1,32 @@
'use strict';

var isNumeric = require('fast-isnumeric');


module.exports = {

// toBeCloseTo... but for arrays
toBeCloseToArray: function() {
return {
compare: function(actual, expected, precision) {
compare: function(actual, expected, precision, msgExtra) {
precision = coercePosition(precision);

var tested = actual.map(function(element, i) {
return Math.abs(expected[i] - element) < precision;
return isClose(element, expected[i], precision);
});

var passed = (
expected.length === actual.length &&
tested.indexOf(false) < 0
);

var message = [
'Expected', actual, 'to be close to', expected, msgExtra
].join(' ');

return {
pass: passed,
message: 'Expected ' + actual + ' to be close to ' + expected + '.'
message: message
};
}
};
Expand All @@ -26,7 +35,7 @@ module.exports = {
// toBeCloseTo... but for 2D arrays
toBeCloseTo2DArray: function() {
return {
compare: function(actual, expected, precision) {
compare: function(actual, expected, precision, msgExtra) {
precision = coercePosition(precision);

var passed = true;
Expand All @@ -40,9 +49,7 @@ module.exports = {
}

for(var j = 0; j < expected[i].length; ++j) {
var isClose = Math.abs(expected[i][j] - actual[i][j]) < precision;

if(!isClose) {
if(!isClose(actual[i][j], expected[i][j], precision)) {
passed = false;
break;
}
Expand All @@ -54,7 +61,8 @@ module.exports = {
'Expected',
arrayToStr(actual.map(arrayToStr)),
'to be close to',
arrayToStr(expected.map(arrayToStr))
arrayToStr(expected.map(arrayToStr)),
msgExtra
].join(' ');

return {
Expand All @@ -66,6 +74,14 @@ module.exports = {
}
};

function isClose(actual, expected, precision) {
if(isNumeric(actual) && isNumeric(expected)) {
return Math.abs(actual - expected) < precision;
}

return actual === expected;
}

function coercePosition(precision) {
if(precision !== 0) {
precision = Math.pow(10, -precision) / 2 || 0.005;
Expand Down
Loading