Skip to content

violin: honor bandwidth attribute even if values in a trace are identical #3626

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 5 commits into from
Mar 19, 2019
Merged

violin: honor bandwidth attribute even if values in a trace are identical #3626

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
121db9c
violin: handle data with zero standard deviation
antoinerg Mar 12, 2019
12a0ab6
violin: defaults to zero width if span is zero
antoinerg Mar 12, 2019
5acb518
violin: improve mock to :lock: handling of bandwidth
antoinerg Mar 12, 2019
7ccdc4e
violin: specify bandwidth to preserve baseline
antoinerg Mar 12, 2019
4698df0
violin: update baselines using original mocks
antoinerg Mar 18, 2019
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
56 changes: 36 additions & 20 deletions src/traces/violin/calc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,24 +37,32 @@ module.exports = function calc(gd, trace) {
var bandwidth = cdi.bandwidth = calcBandwidth(trace, cdi, vals);
var span = cdi.span = calcSpan(trace, cdi, valAxis, bandwidth);

// step that well covers the bandwidth and is multiple of span distance
var dist = span[1] - span[0];
var n = Math.ceil(dist / (bandwidth / 3));
var step = dist / n;

if(!isFinite(step) || !isFinite(n)) {
Lib.error('Something went wrong with computing the violin span');
cd[0].t.empty = true;
return cd;
}

var kde = helpers.makeKDE(cdi, trace, vals);
cdi.density = new Array(n);

for(var k = 0, t = span[0]; t < (span[1] + step / 2); k++, t += step) {
var v = kde(t);
cdi.density[k] = {v: v, t: t};
maxKDE = Math.max(maxKDE, v);
if(cdi.min === cdi.max && bandwidth === 0) {
// if span is zero and bandwidth is zero, we want a violin with zero width
span = cdi.span = [cdi.min, cdi.max];
cdi.density = [{v: 1, t: span[0]}];
cdi.bandwidth = bandwidth;
maxKDE = Math.max(maxKDE, 1);
} else {
// step that well covers the bandwidth and is multiple of span distance
var dist = span[1] - span[0];
var n = Math.ceil(dist / (bandwidth / 3));
var step = dist / n;

if(!isFinite(step) || !isFinite(n)) {
Lib.error('Something went wrong with computing the violin span');
cd[0].t.empty = true;
return cd;
}

var kde = helpers.makeKDE(cdi, trace, vals);
cdi.density = new Array(n);

for(var k = 0, t = span[0]; t < (span[1] + step / 2); k++, t += step) {
var v = kde(t);
cdi.density[k] = {v: v, t: t};
maxKDE = Math.max(maxKDE, v);
}
}

maxCount = Math.max(maxCount, vals.length);
Expand Down Expand Up @@ -100,8 +108,16 @@ function silvermanRule(len, ssd, iqr) {
function calcBandwidth(trace, cdi, vals) {
var span = cdi.max - cdi.min;

// plot single-value violin with bandwidth of 1
if(!span) return 1;
// If span is zero
if(!span) {
if(trace.bandwidth) {
return trace.bandwidth;
} else {
// if span is zero and no bandwidth is specified
// it returns zero bandwidth which is a special case
return 0;
}
}

// Limit how small the bandwidth can be.
//
Expand Down
Binary file added test/image/baselines/violin_only_zeroes.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.
3 changes: 3 additions & 0 deletions test/image/mocks/groups-over-matching-axes.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,9 @@
}
],
"layout": {
"template": {
"data": {"violin": [{"bandwidth": 1}]}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice use of template 🏆

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm. I think I would prefer updating the baselines instead of tweaking the JSON mocks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 4698df0

Copy link
Collaborator

Choose a reason for hiding this comment

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

Not a big deal, but to me the original baselines are a lot easier to look at and understand what's going on. But of course either way it'll serve the function of catching unintended changes.

},
"boxmode": "group",
"violinmode": "group",
"showlegend": false,
Expand Down
3 changes: 3 additions & 0 deletions test/image/mocks/violin-offsetgroups.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@
}
],
"layout": {
"template": {
"data": {"violin": [{"bandwidth": 1}]}
},
"violinmode": "group",
"showlegend": false,
"grid": {
Expand Down
22 changes: 22 additions & 0 deletions test/image/mocks/violin_only_zeroes.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
{
"data": [{
"type": "violin",
"y": [0, 0, 0, 0],
"points": "all",
"pointpos": 0,
"meanline": {
"visible": true
}
}, {
"type": "violin",
"y": [0, 0, 0, 0],
"points": "all",
"bandwidth": 0.25,
"pointpos": 0
}, {
"type": "violin",
"y": [0, 1, 1.5, 2],
"points": "all",
"pointpos": 0
}]
}
4 changes: 2 additions & 2 deletions test/jasmine/tests/violin_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ describe('Test violin calc:', function() {
});

expect(cd.length).toBe(6, '# of violins');
expect(cd.every(function(d) { return d.bandwidth; })).toBe(true, 'bandwidth');
expect(cd.every(function(d) { return d.bandwidth; })).toBe(false, 'bandwidth');
});

it('handle multi-value / single-but-unique-value case', function() {
Expand All @@ -289,7 +289,7 @@ describe('Test violin calc:', function() {
});

expect(cd.length).toBe(1, '# of violins');
expect(cd[0].bandwidth).toBe(1, 'bandwidth');
expect(cd[0].bandwidth).toBe(0, 'bandwidth');
});
});

Expand Down