Skip to content

Commit

Permalink
Merge pull request #2413 from plotly/hist-fix
Browse files Browse the repository at this point in the history 
Fix yet another histogram edge case
  • Loading branch information
@alexcjohnson
alexcjohnson authored Feb 27, 2018
2 parents e0e3ee1 + 619b26d commit efa1275
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 34 deletions.
4 changes: 2 additions & 2 deletions src/plots/cartesian/axes.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -568,7 +568,7 @@ axes.autoBin = function(data, ax, nbins, is2d, calendar) {
start: dataMin - 0.5,
end: dataMax + 0.5,
size: 1,
_count: dataMax - dataMin + 1
_dataSpan: dataMax - dataMin,
};
}

Expand Down Expand Up @@ -648,7 +648,7 @@ axes.autoBin = function(data, ax, nbins, is2d, calendar) {
start: ax.c2r(binStart, 0, calendar),
end: ax.c2r(binEnd, 0, calendar),
size: dummyAx.dtick,
_count: bincount
_dataSpan: dataMax - dataMin
};
};

Expand Down
2 changes: 1 addition & 1 deletion src/traces/histogram/calc.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ function calcAllAutoBins(gd, trace, pa, mainData, _overlayEdgeCase) {

// Edge case: single-valued histogram overlaying others
// Use them all together to calculate the bin size for the single-valued one
if(isOverlay && binSpec._count === 1 && pa.type !== 'category') {
if(isOverlay && binSpec._dataSpan === 0 && pa.type !== 'category') {
// Several single-valued histograms! Stop infinite recursion,
// just return an extra flag that tells handleSingleValueOverlays
// to sort out this trace too
Expand Down
12 changes: 6 additions & 6 deletions test/jasmine/tests/axes_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2369,7 +2369,7 @@ describe('Test axes', function() {
start: -0.5,
end: 2.5,
size: 1,
_count: 3
_dataSpan: 2
});
});

Expand All @@ -2383,7 +2383,7 @@ describe('Test axes', function() {
start: undefined,
end: undefined,
size: 2,
_count: NaN
_dataSpan: NaN
});
});

Expand All @@ -2397,7 +2397,7 @@ describe('Test axes', function() {
start: undefined,
end: undefined,
size: 2,
_count: NaN
_dataSpan: NaN
});
});

Expand All @@ -2411,7 +2411,7 @@ describe('Test axes', function() {
start: undefined,
end: undefined,
size: 2,
_count: NaN
_dataSpan: NaN
});
});

Expand All @@ -2425,7 +2425,7 @@ describe('Test axes', function() {
start: 0.5,
end: 4.5,
size: 1,
_count: 4
_dataSpan: 3
});
});

Expand All @@ -2443,7 +2443,7 @@ describe('Test axes', function() {
start: -0.5,
end: 5.5,
size: 2,
_count: 3
_dataSpan: 3
});
});
});
Expand Down
32 changes: 16 additions & 16 deletions test/jasmine/tests/histogram2d_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -191,42 +191,42 @@ describe('Test histogram2d', function() {
1, 2, 3, 4, 1, 2, 3, 4, 1, 2, 3, 4, 1, 2, 3, 4,
1, 2, 3, 4, 1, 2, 3, 4, 1, 2, 3, 4, 1, 2, 3, 4];
Plotly.newPlot(gd, [{type: 'histogram2d', x: x1, y: y1}]);
expect(gd._fullData[0].xbins).toEqual({start: 0.5, end: 4.5, size: 1, _count: 4});
expect(gd._fullData[0].ybins).toEqual({start: 0.5, end: 4.5, size: 1, _count: 4});
expect(gd._fullData[0].xbins).toEqual({start: 0.5, end: 4.5, size: 1, _dataSpan: 3});
expect(gd._fullData[0].ybins).toEqual({start: 0.5, end: 4.5, size: 1, _dataSpan: 3});
expect(gd._fullData[0].autobinx).toBe(true);
expect(gd._fullData[0].autobiny).toBe(true);

// same range but fewer samples increases sizes
Plotly.restyle(gd, {x: [[1, 3, 4]], y: [[1, 2, 4]]});
expect(gd._fullData[0].xbins).toEqual({start: -0.5, end: 5.5, size: 2, _count: 3});
expect(gd._fullData[0].ybins).toEqual({start: -0.5, end: 5.5, size: 2, _count: 3});
expect(gd._fullData[0].xbins).toEqual({start: -0.5, end: 5.5, size: 2, _dataSpan: 3});
expect(gd._fullData[0].ybins).toEqual({start: -0.5, end: 5.5, size: 2, _dataSpan: 3});
expect(gd._fullData[0].autobinx).toBe(true);
expect(gd._fullData[0].autobiny).toBe(true);

// larger range
Plotly.restyle(gd, {x: [[10, 30, 40]], y: [[10, 20, 40]]});
expect(gd._fullData[0].xbins).toEqual({start: -0.5, end: 59.5, size: 20, _count: 3});
expect(gd._fullData[0].ybins).toEqual({start: -0.5, end: 59.5, size: 20, _count: 3});
expect(gd._fullData[0].xbins).toEqual({start: -0.5, end: 59.5, size: 20, _dataSpan: 30});
expect(gd._fullData[0].ybins).toEqual({start: -0.5, end: 59.5, size: 20, _dataSpan: 30});
expect(gd._fullData[0].autobinx).toBe(true);
expect(gd._fullData[0].autobiny).toBe(true);

// explicit changes to bin settings
Plotly.restyle(gd, 'xbins.start', 12);
expect(gd._fullData[0].xbins).toEqual({start: 12, end: 59.5, size: 20, _count: 3});
expect(gd._fullData[0].ybins).toEqual({start: -0.5, end: 59.5, size: 20, _count: 3});
expect(gd._fullData[0].xbins).toEqual({start: 12, end: 59.5, size: 20, _dataSpan: 30});
expect(gd._fullData[0].ybins).toEqual({start: -0.5, end: 59.5, size: 20, _dataSpan: 30});
expect(gd._fullData[0].autobinx).toBe(false);
expect(gd._fullData[0].autobiny).toBe(true);

Plotly.restyle(gd, {'ybins.end': 12, 'ybins.size': 3});
expect(gd._fullData[0].xbins).toEqual({start: 12, end: 59.5, size: 20, _count: 3});
expect(gd._fullData[0].ybins).toEqual({start: -0.5, end: 12, size: 3, _count: 3});
expect(gd._fullData[0].xbins).toEqual({start: 12, end: 59.5, size: 20, _dataSpan: 30});
expect(gd._fullData[0].ybins).toEqual({start: -0.5, end: 12, size: 3, _dataSpan: 30});
expect(gd._fullData[0].autobinx).toBe(false);
expect(gd._fullData[0].autobiny).toBe(false);

// restart autobin
Plotly.restyle(gd, {autobinx: true, autobiny: true});
expect(gd._fullData[0].xbins).toEqual({start: -0.5, end: 59.5, size: 20, _count: 3});
expect(gd._fullData[0].ybins).toEqual({start: -0.5, end: 59.5, size: 20, _count: 3});
expect(gd._fullData[0].xbins).toEqual({start: -0.5, end: 59.5, size: 20, _dataSpan: 30});
expect(gd._fullData[0].ybins).toEqual({start: -0.5, end: 59.5, size: 20, _dataSpan: 30});
expect(gd._fullData[0].autobinx).toBe(true);
expect(gd._fullData[0].autobiny).toBe(true);
});
Expand All @@ -239,15 +239,15 @@ describe('Test histogram2d', function() {
1, 2, 3, 4, 1, 2, 3, 4, 1, 2, 3, 4, 1, 2, 3, 4,
1, 2, 3, 4, 1, 2, 3, 4, 1, 2, 3, 4, 1, 2, 3, 4];
Plotly.newPlot(gd, [{type: 'histogram2d', x: x1, y: y1, autobinx: false, autobiny: false}]);
expect(gd._fullData[0].xbins).toEqual({start: 0.5, end: 4.5, size: 1, _count: 4});
expect(gd._fullData[0].ybins).toEqual({start: 0.5, end: 4.5, size: 1, _count: 4});
expect(gd._fullData[0].xbins).toEqual({start: 0.5, end: 4.5, size: 1, _dataSpan: 3});
expect(gd._fullData[0].ybins).toEqual({start: 0.5, end: 4.5, size: 1, _dataSpan: 3});
expect(gd._fullData[0].autobinx).toBe(false);
expect(gd._fullData[0].autobiny).toBe(false);

// with autobin false this will no longer update the bins.
Plotly.restyle(gd, {x: [[1, 3, 4]], y: [[1, 2, 4]]});
expect(gd._fullData[0].xbins).toEqual({start: 0.5, end: 4.5, size: 1, _count: 4});
expect(gd._fullData[0].ybins).toEqual({start: 0.5, end: 4.5, size: 1, _count: 4});
expect(gd._fullData[0].xbins).toEqual({start: 0.5, end: 4.5, size: 1, _dataSpan: 3});
expect(gd._fullData[0].ybins).toEqual({start: 0.5, end: 4.5, size: 1, _dataSpan: 3});
expect(gd._fullData[0].autobinx).toBe(false);
expect(gd._fullData[0].autobiny).toBe(false);
});
Expand Down
38 changes: 29 additions & 9 deletions test/jasmine/tests/histogram_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,15 @@ describe('Test histogram', function() {

var out = calc(gd, fullTrace);
delete out[0].trace;

// this is dumb - but some of the `p0` values are `-0` which doesn't match `0`
// even though -0 === 0
out.forEach(function(cdi) {
for(var key in cdi) {
if(cdi[key] === 0) cdi[key] = 0;
}
});

return out;
}

Expand Down Expand Up @@ -238,14 +247,6 @@ describe('Test histogram', function() {
nbinsx: 4
});

// this is dumb - but some of the `p0` values are `-0` which doesn't match `0`
// even though -0 === 0
out.forEach(function(cdi) {
Object.keys(cdi).forEach(function(key) {
if(cdi[key] === 0) cdi[key] = 0;
});
});

expect(out).toEqual([
// dec 31 12:00 -> jan 31 12:00, middle is jan 16
{i: 0, b: 0, p: Date.UTC(1970, 0, 16), s: 2, pts: [0, 1], p0: Date.UTC(1970, 0, 1), p1: Date.UTC(1970, 0, 31)},
Expand Down Expand Up @@ -365,6 +366,25 @@ describe('Test histogram', function() {
]);
});

it('can tell the difference between single-bin and single-value histograms', function() {
var out = _calc({x: [1, 4]}, [], {barmode: 'overlay'});

expect(out).toEqual([
{i: 0, b: 0, p: 2, s: 2, width1: 5, pts: [0, 1], p0: 0, p1: 4}
]);

// real single-valued trace inherits bar width from the simply single-bin trace
out = _calc({x: [5]}, [
{x: [1, 4]}
], {
barmode: 'overlay'
});

expect(out).toEqual([
{i: 0, b: 0, p: 5, s: 1, width1: 5, pts: [0], p0: 5, p1: 5}
]);
});

function calcPositions(opts, extraTraces) {
return _calc(opts, extraTraces).map(function(v) { return v.p; });
}
Expand Down Expand Up @@ -652,7 +672,7 @@ describe('Test histogram', function() {
.then(done);
});

it('give the right bar width for single-bin histograms', function(done) {
it('gives the right bar width for single-value histograms', function(done) {
Plotly.newPlot(gd, [{
type: 'histogram',
x: [3, 3, 3],
Expand Down

0 comments on commit efa1275

Please sign in to comment.