Skip to content

copy histogram autobinx/y back to the input trace #1901

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
Jul 20, 2017
Merged

copy histogram autobinx/y back to the input trace #1901

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: 10 additions & 6 deletions src/traces/histogram/calc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,11 @@ module.exports = function calc(gd, trace) {
var pos0 = pa.makeCalcdata(trace, maindata);

// calculate the bins
var binAttr = maindata + 'bins',
binspec;
if((trace['autobin' + maindata] !== false) || !(binAttr in trace)) {
var binAttr = maindata + 'bins';
var autoBinAttr = 'autobin' + maindata;
var binspec = trace[binAttr];
if((trace[autoBinAttr] !== false) || !binspec ||
binspec.start === null || binspec.end === null) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

note that cleanBins provides a size even if there are no start or end. This also doesn't allow you to specify a start and no end or things like that. So partial bin definition might do strange things... but that I would definitely consider a feature we could propose to add, rather than a bug to fix.

Also note that !(binAttr in trace) in the old version never failed, since we default to null for start and end (which might have been important to enable cleanBins?) - so in principle I could omit !binspec here, just thought it prudent in case there's some context I'm not aware of that can call this calc.

binspec = Axes.autoBin(pos0, pa, trace['nbins' + maindata], false, calendar);

// adjust for CDF edge cases
Expand All @@ -60,9 +62,11 @@ module.exports = function calc(gd, trace) {

// copy bin info back to the source and full data.
trace._input[binAttr] = trace[binAttr] = binspec;
}
else {
binspec = trace[binAttr];
// note that it's possible to get here with an explicit autobin: false
// if the bins were not specified.
// in that case this will remain in the trace, so that future updates
// which would change the autobinning will not do so.
trace._input[autoBinAttr] = trace[autoBinAttr];
}

var nonuniformBins = typeof binspec.size === 'string',
Expand Down
12 changes: 10 additions & 2 deletions src/traces/histogram2d/calc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@ module.exports = function calc(gd, trace) {


// calculate the bins
if(trace.autobinx || !('xbins' in trace)) {
if(trace.autobinx || !trace.xbins ||
trace.xbins.start === null || trace.xbins.end === null) {
trace.xbins = Axes.autoBin(x, xa, trace.nbinsx, '2d', xcalendar);
if(trace.type === 'histogram2dcontour') {
// the "true" last argument reverses the tick direction (which we can't
Expand All @@ -58,8 +59,14 @@ module.exports = function calc(gd, trace) {

// copy bin info back to the source data.
trace._input.xbins = trace.xbins;
// note that it's possible to get here with an explicit autobin: false
// if the bins were not specified.
// in that case this will remain in the trace, so that future updates
// which would change the autobinning will not do so.
trace._input.autobinx = trace.autobinx;
}
if(trace.autobiny || !('ybins' in trace)) {
if(trace.autobiny || !trace.ybins ||
trace.ybins.start === null || trace.ybins.end === null) {
trace.ybins = Axes.autoBin(y, ya, trace.nbinsy, '2d', ycalendar);
if(trace.type === 'histogram2dcontour') {
trace.ybins.start = yc2r(Axes.tickIncrement(
Expand All @@ -68,6 +75,7 @@ module.exports = function calc(gd, trace) {
yr2c(trace.ybins.end), trace.ybins.size, false, ycalendar));
}
trace._input.ybins = trace.ybins;
trace._input.autobiny = trace.autobiny;
}

// make the empty bin array & scale the map
Expand Down
79 changes: 76 additions & 3 deletions test/jasmine/tests/histogram2d_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,13 +137,17 @@ describe('Test histogram2d', function() {
});
});

describe('relayout interaction', function() {
describe('restyle / relayout interaction', function() {

var gd;

beforeEach(function() {
gd = createGraphDiv();
});

afterEach(destroyGraphDiv);

it('should update paths on zooms', function(done) {
var gd = createGraphDiv();

Plotly.newPlot(gd, [{
type: 'histogram2dcontour',
x: [1, 1, 2, 2, 3],
Expand All @@ -156,6 +160,75 @@ describe('Test histogram2d', function() {
.then(done);
});


it('handles autobin correctly on restyles', function() {
var x1 = [
1, 1, 1, 1, 2, 2, 2, 2, 3, 3, 3, 3, 4, 4, 4, 4,
1, 1, 1, 1, 2, 2, 2, 2, 3, 3, 3, 3, 4, 4, 4, 4];
var y1 = [
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].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].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].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].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].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].autobinx).toBe(true);
expect(gd._fullData[0].autobiny).toBe(true);
});

it('respects explicit autobin: false as a one-time autobin', function() {
Copy link
Contributor

Choose a reason for hiding this comment

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

That's cool 👍

var x1 = [
1, 1, 1, 1, 2, 2, 2, 2, 3, 3, 3, 3, 4, 4, 4, 4,
1, 1, 1, 1, 2, 2, 2, 2, 3, 3, 3, 3, 4, 4, 4, 4];
var y1 = [
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].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].autobinx).toBe(false);
expect(gd._fullData[0].autobiny).toBe(false);
});
});

});
59 changes: 59 additions & 0 deletions test/jasmine/tests/histogram_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
var Plotly = require('@lib/index');
var Plots = require('@src/plots/plots');
var Lib = require('@src/lib');

var supplyDefaults = require('@src/traces/histogram/defaults');
var calc = require('@src/traces/histogram/calc');

var createGraphDiv = require('../assets/create_graph_div');
var destroyGraphDiv = require('../assets/destroy_graph_div');


describe('Test histogram', function() {
'use strict';
Expand Down Expand Up @@ -365,4 +369,59 @@ describe('Test histogram', function() {
});

});

describe('plot / restyle', function() {
var gd;

beforeEach(function() {
gd = createGraphDiv();
});

afterEach(destroyGraphDiv);

it('should update autobins correctly when restyling', function() {
// note: I'm *not* testing what this does to gd.data, as that's
// really a matter of convenience and will perhaps change later (v2?)
var data1 = [1.5, 2, 2, 3, 3, 3, 4, 4, 5];
Plotly.plot(gd, [{x: data1, type: 'histogram' }]);
expect(gd._fullData[0].xbins).toEqual({start: 1, end: 6, size: 1});
expect(gd._fullData[0].autobinx).toBe(true);

// same range but fewer samples changes autobin size
var data2 = [1.5, 5];
Plotly.restyle(gd, 'x', [data2]);
expect(gd._fullData[0].xbins).toEqual({start: -2.5, end: 7.5, size: 5});
expect(gd._fullData[0].autobinx).toBe(true);

// different range
var data3 = [10, 20.2, 20, 30, 30, 30, 40, 40, 50];
Plotly.restyle(gd, 'x', [data3]);
expect(gd._fullData[0].xbins).toEqual({start: 5, end: 55, size: 10});
expect(gd._fullData[0].autobinx).toBe(true);

// explicit change to a bin attribute clears autobin
Plotly.restyle(gd, 'xbins.start', 3);
expect(gd._fullData[0].xbins).toEqual({start: 3, end: 55, size: 10});
expect(gd._fullData[0].autobinx).toBe(false);

// restart autobin
Plotly.restyle(gd, 'autobinx', true);
expect(gd._fullData[0].xbins).toEqual({start: 5, end: 55, size: 10});
expect(gd._fullData[0].autobinx).toBe(true);
});

it('respects explicit autobin: false as a one-time autobin', function() {
var data1 = [1.5, 2, 2, 3, 3, 3, 4, 4, 5];
Plotly.plot(gd, [{x: data1, type: 'histogram', autobinx: false }]);
// we have no bins, so even though autobin is false we have to autobin once
expect(gd._fullData[0].xbins).toEqual({start: 1, end: 6, size: 1});
expect(gd._fullData[0].autobinx).toBe(false);

// since autobin is false, this will not change the bins
var data2 = [1.5, 5];
Plotly.restyle(gd, 'x', [data2]);
expect(gd._fullData[0].xbins).toEqual({start: 1, end: 6, size: 1});
expect(gd._fullData[0].autobinx).toBe(false);
});
});
});