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.

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

Jump to bottom

Histogram edge cases #2028

Merged
merged 5 commits into from Sep 22, 2017
Merged

Histogram edge cases #2028

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a9498bb
fix part 1 of #1978 - keep bin width and trim zeros from 1-bin histog…
alexcjohnson Sep 20, 2017
968f742
fix part 2 of #1978 - autobin size for single-value overlaid histograms
alexcjohnson Sep 21, 2017
9959341
fix histogram2d test for _count field
alexcjohnson Sep 21, 2017
bcebc8a
fix sieve modification -> for single-item bar traces
alexcjohnson Sep 21, 2017
c057d25
lint
alexcjohnson Sep 21, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 9 additions & 5 deletions src/plots/cartesian/axes.js
View file
Open in desktop
Expand Up @@ -569,7 +569,8 @@ axes.autoBin = function(data, ax, nbins, is2d, calendar) {
return {
start: dataMin - 0.5,
end: dataMax + 0.5,
size: 1
size: 1,
_count: dataMax - dataMin + 1
};
}

Expand Down Expand Up @@ -613,16 +614,16 @@ axes.autoBin = function(data, ax, nbins, is2d, calendar) {

axes.autoTicks(dummyAx, size0);
var binStart = axes.tickIncrement(
axes.tickFirst(dummyAx), dummyAx.dtick, 'reverse', calendar),
binEnd;
axes.tickFirst(dummyAx), dummyAx.dtick, 'reverse', calendar);
var binEnd, bincount;

// check for too many data points right at the edges of bins
// (>50% within 1% of bin edges) or all data points integral
// and offset the bins accordingly
if(typeof dummyAx.dtick === 'number') {
binStart = autoShiftNumericBins(binStart, data, dummyAx, dataMin, dataMax);

var bincount = 1 + Math.floor((dataMax - binStart) / dummyAx.dtick);
bincount = 1 + Math.floor((dataMax - binStart) / dummyAx.dtick);
binEnd = binStart + bincount * dummyAx.dtick;
}
else {
Expand All @@ -638,15 +639,18 @@ axes.autoBin = function(data, ax, nbins, is2d, calendar) {
// calculate the endpoint for nonlinear ticks - you have to
// just increment until you're done
binEnd = binStart;
bincount = 0;
while(binEnd <= dataMax) {
binEnd = axes.tickIncrement(binEnd, dummyAx.dtick, false, calendar);
bincount++;
}
}

return {
start: ax.c2r(binStart, 0, calendar),
end: ax.c2r(binEnd, 0, calendar),
size: dummyAx.dtick
size: dummyAx.dtick,
_count: bincount
};
};

Expand Down
11 changes: 9 additions & 2 deletions src/traces/bar/sieve.js
View file
Open in desktop
Expand Up @@ -30,19 +30,26 @@ function Sieve(traces, separateNegativeValues, dontMergeOverlappingData) {
this.separateNegativeValues = separateNegativeValues;
this.dontMergeOverlappingData = dontMergeOverlappingData;

// for single-bin histograms - see histogram/calc
var width1 = Infinity;

var positions = [];
for(var i = 0; i < traces.length; i++) {
var trace = traces[i];
for(var j = 0; j < trace.length; j++) {
var bar = trace[j];
if(bar.p !== BADNUM) positions.push(bar.p);
}
if(trace[0] && trace[0].width1) {
width1 = Math.min(trace[0].width1, width1);
}
}
this.positions = positions;

var dv = Lib.distinctVals(this.positions);
var dv = Lib.distinctVals(positions);
this.distinctPositions = dv.vals;
this.minDiff = dv.minDiff;
if(dv.vals.length === 1 && width1 !== Infinity) this.minDiff = width1;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@n-riesco you wrote some fantastic tests of bar chart edge cases, really saved us here!

else this.minDiff = Math.min(dv.minDiff, width1);

this.binWidth = this.minDiff;

Expand Down
118 changes: 108 additions & 10 deletions src/traces/histogram/calc.js
View file
Open in desktop
Expand Up @@ -135,7 +135,7 @@ module.exports = function calc(gd, trace) {
break;
}
}
for(i = seriesLen - 1; i > firstNonzero; i--) {
for(i = seriesLen - 1; i >= firstNonzero; i--) {
if(size[i]) {
lastNonzero = i;
break;
Expand All @@ -149,6 +149,12 @@ module.exports = function calc(gd, trace) {
}
}

if(cd.length === 1) {
// when we collapse to a single bin, calcdata no longer describes bin size
// so we need to explicitly specify it
cd[0].width1 = Axes.tickIncrement(cd[0].p, binSpec.size, false, calendar) - cd[0].p;
}

arraysToCalcdata(cd, trace);

return cd;
Expand All @@ -161,8 +167,9 @@ module.exports = function calc(gd, trace) {
* smallest bins of any of the auto values for all histograms grouped/stacked
* together.
*/
function calcAllAutoBins(gd, trace, pa, mainData) {
function calcAllAutoBins(gd, trace, pa, mainData, _overlayEdgeCase) {
var binAttr = mainData + 'bins';
var isOverlay = gd._fullLayout.barmode === 'overlay';
var i, tracei, calendar, firstManual, pos0;

// all but the first trace in this group has already been marked finished
Expand All @@ -172,7 +179,9 @@ function calcAllAutoBins(gd, trace, pa, mainData) {
}
else {
// must be the first trace in the group - do the autobinning on them all
var traceGroup = getConnectedHistograms(gd, trace);

// find all grouped traces - in overlay mode each trace is independent
var traceGroup = isOverlay ? [trace] : getConnectedHistograms(gd, trace);
var autoBinnedTraces = [];

var minSize = Infinity;
Expand All @@ -196,6 +205,19 @@ function calcAllAutoBins(gd, trace, pa, mainData) {

binSpec = Axes.autoBin(pos0, pa, tracei['nbins' + mainData], false, calendar);

// 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') {
// trace[binAttr] = binSpec;
Copy link
Contributor

Choose a reason for hiding this comment

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

🔪 ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

thanks - 🔪 in c057d25


// Several single-valued histograms! Stop infinite recursion,
// just return an extra flag that tells handleSingleValueOverlays
// to sort out this trace too
if(_overlayEdgeCase) return [binSpec, pos0, true];

binSpec = handleSingleValueOverlays(gd, trace, pa, mainData, binAttr);
}

// adjust for CDF edge cases
if(cumulativeSpec.enabled && (cumulativeSpec.currentbin !== 'include')) {
if(cumulativeSpec.direction === 'decreasing') {
Expand All @@ -212,9 +234,9 @@ function calcAllAutoBins(gd, trace, pa, mainData) {
}
else if(!firstManual) {
// Remember the first manually set binSpec. We'll try to be extra
// accommodating of this one, so other bins line up with these
// if there's more than one manual bin set and they're mutually inconsistent,
// then there's not much we can do...
// accommodating of this one, so other bins line up with these.
// But if there's more than one manual bin set and they're mutually
// inconsistent, then there's not much we can do...
firstManual = {
size: binSpec.size,
start: pa.r2c(binSpec.start, 0, calendar),
Expand Down Expand Up @@ -276,14 +298,90 @@ function calcAllAutoBins(gd, trace, pa, mainData) {
}

/*
* Return an array of traces that are all stacked or grouped together
* Only considers histograms. In principle we could include them in a
* Adjust single-value histograms in overlay mode to make as good a
* guess as we can at autobin values the user would like.
*
* Returns the binSpec for the trace that sparked all this
*/
function handleSingleValueOverlays(gd, trace, pa, mainData, binAttr) {
var overlaidTraceGroup = getConnectedHistograms(gd, trace);
var pastThisTrace = false;
var minSize = Infinity;
var singleValuedTraces = [trace];
var i, tracei;

// first collect all the:
// - min bin size from all multi-valued traces
// - single-valued traces
for(i = 0; i < overlaidTraceGroup.length; i++) {
tracei = overlaidTraceGroup[i];
if(tracei === trace) pastThisTrace = true;
else if(!pastThisTrace) {
// This trace has already had its autobins calculated
// (so must not have been single-valued).
minSize = Math.min(minSize, tracei[binAttr].size);
}
else {
var resulti = calcAllAutoBins(gd, tracei, pa, mainData, true);
var binSpeci = resulti[0];
var isSingleValued = resulti[2];

// so we can use this result when we get to tracei in the normal
// course of events, mark it as done and put _pos0 back
tracei._autoBinFinished = 1;
tracei._pos0 = resulti[1];

if(isSingleValued) {
singleValuedTraces.push(tracei);
}
else {
minSize = Math.min(minSize, binSpeci.size);
}
}
}

// find the real data values for each single-valued trace
// hunt through pos0 for the first valid value
var dataVals = new Array(singleValuedTraces.length);
for(i = 0; i < singleValuedTraces.length; i++) {
var pos0 = singleValuedTraces[i]._pos0;
for(var j = 0; j < pos0.length; j++) {
if(pos0[j] !== undefined) {
dataVals[i] = pos0[j];
break;
}
}
}

// are ALL traces are single-valued? use the min difference between
// all of their values (which defaults to 1 if there's still only one)
if(!isFinite(minSize)) {
minSize = Lib.distinctVals(dataVals).minDiff;
}

// now apply the min size we found to all single-valued traces
for(i = 0; i < singleValuedTraces.length; i++) {
tracei = singleValuedTraces[i];
var calendar = tracei[mainData + 'calendar'];

tracei._input[binAttr] = tracei[binAttr] = {
start: pa.c2r(dataVals[i] - minSize / 2, 0, calendar),
end: pa.c2r(dataVals[i] + minSize / 2, 0, calendar),
size: minSize
};
}

return trace[binAttr];
}

/*
* Return an array of histograms that share axes and orientation.
*
* Only considers histograms. In principle we could include bars in a
* similar way to how we do manually binned histograms, though this
* would have tons of edge cases and value judgments to make.
*/
function getConnectedHistograms(gd, trace) {
if(gd._fullLayout.barmode === 'overlay') return [trace];

var xid = trace.xaxis;
var yid = trace.yaxis;
var orientation = trace.orientation;
Expand Down
18 changes: 12 additions & 6 deletions test/jasmine/tests/axes_test.js
View file
Open in desktop
Expand Up @@ -2401,7 +2401,8 @@ describe('Test axes', function() {
expect(out).toEqual({
start: -0.5,
end: 2.5,
size: 1
size: 1,
_count: 3
});
});

Expand All @@ -2414,7 +2415,8 @@ describe('Test axes', function() {
expect(out).toEqual({
start: undefined,
end: undefined,
size: 2
size: 2,
_count: NaN
});
});

Expand All @@ -2427,7 +2429,8 @@ describe('Test axes', function() {
expect(out).toEqual({
start: undefined,
end: undefined,
size: 2
size: 2,
_count: NaN
});
});

Expand All @@ -2440,7 +2443,8 @@ describe('Test axes', function() {
expect(out).toEqual({
start: undefined,
end: undefined,
size: 2
size: 2,
_count: NaN
});
});

Expand All @@ -2453,7 +2457,8 @@ describe('Test axes', function() {
expect(out).toEqual({
start: 0.5,
end: 4.5,
size: 1
size: 1,
_count: 4
});
});

Expand All @@ -2470,7 +2475,8 @@ describe('Test axes', function() {
expect(out).toEqual({
start: -0.5,
end: 5.5,
size: 2
size: 2,
_count: 3
});
});
});
Expand Down
32 changes: 16 additions & 16 deletions test/jasmine/tests/histogram2d_test.js
View file
Open in desktop
Expand Up @@ -169,42 +169,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});
expect(gd._fullData[0].ybins).toEqual({start: 0.5, end: 4.5, size: 1});
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].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});
expect(gd._fullData[0].ybins).toEqual({start: -0.5, end: 5.5, size: 2});
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].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});
expect(gd._fullData[0].ybins).toEqual({start: -0.5, end: 59.5, size: 20});
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].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});
expect(gd._fullData[0].ybins).toEqual({start: -0.5, end: 59.5, size: 20});
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].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});
expect(gd._fullData[0].ybins).toEqual({start: -0.5, end: 12, 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].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});
expect(gd._fullData[0].ybins).toEqual({start: -0.5, end: 59.5, size: 20});
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].autobinx).toBe(true);
expect(gd._fullData[0].autobiny).toBe(true);
});
Expand All @@ -217,15 +217,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});
expect(gd._fullData[0].ybins).toEqual({start: 0.5, end: 4.5, size: 1});
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].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});
expect(gd._fullData[0].ybins).toEqual({start: 0.5, end: 4.5, size: 1});
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].autobinx).toBe(false);
expect(gd._fullData[0].autobiny).toBe(false);
});
Expand Down