Skip to content
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

Unhide overlapping grouped bars within trace #3680

Merged
merged 3 commits into from
Mar 28, 2019
Merged
Show file tree
Hide file tree
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
145 changes: 95 additions & 50 deletions src/traces/bar/cross_trace_calc.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,52 +57,57 @@ function setGroupPositions(gd, pa, sa, calcTraces) {
if(!calcTraces.length) return;

var barmode = gd._fullLayout.barmode;
var overlay = (barmode === 'overlay');
var group = (barmode === 'group');
var excluded;
var included;
var i, calcTrace, fullTrace;

initBase(gd, pa, sa, calcTraces);

if(overlay) {
setGroupPositionsInOverlayMode(gd, pa, sa, calcTraces);
} else if(group) {
// exclude from the group those traces for which the user set an offset
excluded = [];
included = [];
for(i = 0; i < calcTraces.length; i++) {
calcTrace = calcTraces[i];
fullTrace = calcTrace[0].trace;

if(fullTrace.offset === undefined) included.push(calcTrace);
else excluded.push(calcTrace);
}
switch(barmode) {
case 'overlay':
setGroupPositionsInOverlayMode(gd, pa, sa, calcTraces);
break;

case 'group':
// exclude from the group those traces for which the user set an offset
excluded = [];
included = [];
for(i = 0; i < calcTraces.length; i++) {
calcTrace = calcTraces[i];
fullTrace = calcTrace[0].trace;

if(fullTrace.offset === undefined) included.push(calcTrace);
else excluded.push(calcTrace);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we put this logic in a function and reuse it for the case of relative below?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll pass if you don't mind. That would a pretty ugly function (we would have to pass functions setGroupPositionsInGroupMode, etc) and only save a few lines.


if(included.length) {
setGroupPositionsInGroupMode(gd, pa, sa, included);
}
if(excluded.length) {
setGroupPositionsInOverlayMode(gd, pa, sa, excluded);
}
} else {
// exclude from the stack those traces for which the user set a base
excluded = [];
included = [];
for(i = 0; i < calcTraces.length; i++) {
calcTrace = calcTraces[i];
fullTrace = calcTrace[0].trace;

if(fullTrace.base === undefined) included.push(calcTrace);
else excluded.push(calcTrace);
}
if(included.length) {
setGroupPositionsInGroupMode(gd, pa, sa, included);
}
if(excluded.length) {
setGroupPositionsInOverlayMode(gd, pa, sa, excluded);
}
break;

case 'stack':
case 'relative':
// exclude from the stack those traces for which the user set a base
excluded = [];
included = [];
for(i = 0; i < calcTraces.length; i++) {
calcTrace = calcTraces[i];
fullTrace = calcTrace[0].trace;

if(fullTrace.base === undefined) included.push(calcTrace);
else excluded.push(calcTrace);
}

if(included.length) {
setGroupPositionsInStackOrRelativeMode(gd, pa, sa, included);
}
if(excluded.length) {
setGroupPositionsInOverlayMode(gd, pa, sa, excluded);
}
if(included.length) {
setGroupPositionsInStackOrRelativeMode(gd, pa, sa, included);
}
if(excluded.length) {
setGroupPositionsInOverlayMode(gd, pa, sa, excluded);
}
break;
}

collectExtents(calcTraces, pa);
Expand Down Expand Up @@ -154,13 +159,15 @@ function initBase(gd, pa, sa, calcTraces) {

function setGroupPositionsInOverlayMode(gd, pa, sa, calcTraces) {
var barnorm = gd._fullLayout.barnorm;
var separateNegativeValues = false;
var dontMergeOverlappingData = !barnorm;

// update position axis and set bar offsets and widths
for(var i = 0; i < calcTraces.length; i++) {
var calcTrace = calcTraces[i];
var sieve = new Sieve([calcTrace], separateNegativeValues, dontMergeOverlappingData);

var sieve = new Sieve([calcTrace], {
sepNegVal: false,
overlapNoMerge: !barnorm
});

// set bar offsets and widths, and update position axis
setOffsetAndWidth(gd, pa, sieve);
Expand All @@ -182,13 +189,19 @@ function setGroupPositionsInOverlayMode(gd, pa, sa, calcTraces) {
function setGroupPositionsInGroupMode(gd, pa, sa, calcTraces) {
var fullLayout = gd._fullLayout;
var barnorm = fullLayout.barnorm;
var separateNegativeValues = false;
var dontMergeOverlappingData = !barnorm;
var sieve = new Sieve(calcTraces, separateNegativeValues, dontMergeOverlappingData);

var sieve = new Sieve(calcTraces, {
sepNegVal: false,
overlapNoMerge: !barnorm
});

// set bar offsets and widths, and update position axis
setOffsetAndWidthInGroupMode(gd, pa, sieve);

// relative-stack bars within the same trace that would otherwise
// be hidden
unhideBarsWithinTrace(gd, sa, sieve);

// set bar bases and sizes, and update size axis
if(barnorm) {
sieveBars(gd, sa, sieve);
Expand All @@ -201,12 +214,12 @@ function setGroupPositionsInGroupMode(gd, pa, sa, calcTraces) {
function setGroupPositionsInStackOrRelativeMode(gd, pa, sa, calcTraces) {
var fullLayout = gd._fullLayout;
var barmode = fullLayout.barmode;
var stack = barmode === 'stack';
var relative = barmode === 'relative';
var barnorm = fullLayout.barnorm;
var separateNegativeValues = relative;
var dontMergeOverlappingData = !(barnorm || stack || relative);
var sieve = new Sieve(calcTraces, separateNegativeValues, dontMergeOverlappingData);

var sieve = new Sieve(calcTraces, {
sepNegVal: barmode === 'relative',
overlapNoMerge: !(barnorm || barmode === 'stack' || barmode === 'relative')
});

// set bar offsets and widths, and update position axis
setOffsetAndWidth(gd, pa, sieve);
Expand Down Expand Up @@ -562,7 +575,39 @@ function sieveBars(gd, sa, sieve) {
for(var j = 0; j < calcTrace.length; j++) {
var bar = calcTrace[j];

if(bar.s !== BADNUM) sieve.put(bar.p, bar.b + bar.s);
if(bar.s !== BADNUM) {
sieve.put(bar.p, bar.b + bar.s);
}
}
}
}

function unhideBarsWithinTrace(gd, sa, sieve) {
var calcTraces = sieve.traces;

for(var i = 0; i < calcTraces.length; i++) {
var calcTrace = calcTraces[i];
var fullTrace = calcTrace[0].trace;

if(fullTrace.base === undefined) {
var inTraceSieve = new Sieve([calcTrace], {
sepNegVal: true,
overlapNoMerge: true
});

for(var j = 0; j < calcTrace.length; j++) {
var bar = calcTrace[j];

if(bar.p !== BADNUM) {
// stack current bar and get previous sum
var barBase = inTraceSieve.put(bar.p, bar.b + bar.s);

// if previous sum if non-zero, this means:
// multiple bars have same starting point are potentially hidden,
// shift them vertically so that all bars are visible by default
if(barBase) bar.b = barBase;
}
}
}
}
}
Expand Down
32 changes: 17 additions & 15 deletions src/traces/bar/sieve.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,20 @@ var BADNUM = require('../../constants/numerical').BADNUM;
* Helper class to sieve data from traces into bins
*
* @class
* @param {Array} traces
* Array of calculated traces
* @param {boolean} [separateNegativeValues]
* If true, then split data at the same position into a bar
* for positive values and another for negative values
* @param {boolean} [dontMergeOverlappingData]
* If true, then don't merge overlapping bars into a single bar
*
* @param {Array} traces
* Array of calculated traces
* @param {object} opts
* - @param {boolean} [sepNegVal]
* If true, then split data at the same position into a bar
* for positive values and another for negative values
* - @param {boolean} [overlapNoMerge]
* If true, then don't merge overlapping bars into a single bar
*/
function Sieve(traces, separateNegativeValues, dontMergeOverlappingData) {
function Sieve(traces, opts) {
this.traces = traces;
this.separateNegativeValues = separateNegativeValues;
this.dontMergeOverlappingData = dontMergeOverlappingData;
this.sepNegVal = opts.sepNegVal;
this.overlapNoMerge = opts.overlapNoMerge;

// for single-bin histograms - see histogram/calc
var width1 = Infinity;
Expand Down Expand Up @@ -79,7 +81,7 @@ Sieve.prototype.put = function put(position, value) {
* @method
* @param {number} position Position of datum
* @param {number} [value] Value of datum
* (required if this.separateNegativeValues is true)
* (required if this.sepNegVal is true)
* @returns {number} Current bin value
*/
Sieve.prototype.get = function put(position, value) {
Expand All @@ -93,14 +95,14 @@ Sieve.prototype.get = function put(position, value) {
* @method
* @param {number} position Position of datum
* @param {number} [value] Value of datum
* (required if this.separateNegativeValues is true)
* (required if this.sepNegVal is true)
* @returns {string} Bin label
* (prefixed with a 'v' if value is negative and this.separateNegativeValues is
* (prefixed with a 'v' if value is negative and this.sepNegVal is
* true; otherwise prefixed with '^')
*/
Sieve.prototype.getLabel = function getLabel(position, value) {
var prefix = (value < 0 && this.separateNegativeValues) ? 'v' : '^';
var label = (this.dontMergeOverlappingData) ?
var prefix = (value < 0 && this.sepNegVal) ? 'v' : '^';
var label = (this.overlapNoMerge) ?
position :
Math.round(position / this.binWidth);
return prefix + label;
Expand Down
Binary file added test/image/baselines/bar_unhidden.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
27 changes: 27 additions & 0 deletions test/image/mocks/bar_unhidden.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
{
"data": [
{
"type": "bar",
"x": [ 0, 0, 0 ],
"y": [ 1, 1, -1 ],
"marker": {
"color": [ "red", "green", "blue" ]
}
},
{
"type": "bar",
"x": [ 0, 0, 0 ],
"y": [ 1, 1, -1 ],
"marker": {
"color": [ "cyan", "magenta", "yellow" ]
}
}
],
"layout": {
"margin": { "t": 30, "b": 30, "l": 30, "r": 30 },
"width": 400,
"height": 400,
"showlegend": false,
"hovermode": "closest"
}
}
61 changes: 61 additions & 0 deletions test/jasmine/tests/bar_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -926,6 +926,67 @@ describe('Bar.crossTraceCalc (formerly known as setPositions)', function() {
expect(gd._fullLayout.xaxis.type).toBe('multicategory');
assertPointField(gd.calcdata, 'b', [[0, 0, 0, 0]]);
});

describe('should relative-stack bar within the same trace that overlap under barmode=group', function() {
it('- base case', function() {
var gd = mockBarPlot([{
x: [0, 0, 0],
y: [1, -2, -1]
}]);

assertPointField(gd.calcdata, 'b', [[0, 0, -2]]);
assertPointField(gd.calcdata, 'y', [[1, -2, -3]]);
});

it('- with blank positions', function() {
var gd = mockBarPlot([{
x: [0, null, 0, null, 0],
y: [1, null, -2, null, -1]
}]);

assertPointField(gd.calcdata, 'b', [[0, 0, 0, 0, -2]]);
assertPointField(gd.calcdata, 'y', [[1, NaN, -2, NaN, -3]]);
});

it('- with barnorm set', function() {
var gd = mockBarPlot([{
x: [0, 0, 0],
y: [1, -2, -1],
}], {
barnorm: 'fraction'
});

assertPointField(gd.calcdata, 'b', [[0, 0, -0.5]]);
assertPointField(gd.calcdata, 'y', [[0.25, -0.5, -0.75]]);
});

it('- skipped when base is set', function() {
var gd = mockBarPlot([{
x: [0, 0, 0],
y: [1, -2, -1],
base: 10
}, {
x: [0, 0, 0],
y: [1, -2, -1],
base: [1, 2, 1]
}]);

assertPointField(gd.calcdata, 'b', [[10, 10, 10], [1, 2, 1]]);
assertPointField(gd.calcdata, 'y', [[11, 8, 9], [2, 0, 0]]);
});

it('- skipped when barmode=overlay', function() {
var gd = mockBarPlot([{
x: [0, 0, 0],
y: [1, -2, -1]
}], {
barmode: 'overlay'
});

assertPointField(gd.calcdata, 'b', [[0, 0, 0]]);
assertPointField(gd.calcdata, 'y', [[1, -2, -1]]);
});
});
});

describe('A bar plot', function() {
Expand Down