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

Implement notched box plots, closes #2286 #2305

Merged
merged 7 commits into from
Jan 30, 2018
Merged

Implement notched box plots, closes #2286 #2305

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
a234668
Implement notched box plots, closes #2286
krassowski Jan 27, 2018
144989c
Address 3/4 of @alexcjohnson's comments on notches
krassowski Jan 29, 2018
0eb99e0
Merge branch 'master' into notched_box_plot
krassowski Jan 30, 2018
453d329
Implement 'smarter' notched/notchwidth default
krassowski Jan 30, 2018
e621d0f
Change notchwidth convention to Matlab, d=0.25
krassowski Jan 30, 2018
4ffc52b
Set notch width range to (0, 0.5) instead of (0,1)
krassowski Jan 30, 2018
8dde2d2
Update test images for notched box plots
krassowski Jan 30, 2018
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
21 changes: 21 additions & 0 deletions src/traces/box/attributes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,27 @@ module.exports = {
'For example, with 1, the whiskers are as wide as the box(es).'
].join(' ')
},
notched: {
valType: 'boolean',
role: 'style',
editType: 'calcIfAutorange',
description: [
'Determines whether or not notches should be drawn.'
].join(' ')
},
notchwidth: {
valType: 'number',
min: 0,
max: 1,
Copy link
Collaborator

Choose a reason for hiding this comment

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

also in the matlab convention the max is 0.5

dflt: 0.25,
role: 'style',
editType: 'calcIfAutorange',
description: [
'Sets the width of the notches inversely proportional to',
'the box\' width.',
'For example, with 0, the notches are as wide as the box(es).'
].join(' ')
},
boxpoints: {
valType: 'enumerated',
values: ['all', 'outliers', 'suspectedoutliers', false],
Expand Down
7 changes: 7 additions & 0 deletions src/traces/box/calc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,13 @@ module.exports = function calc(gd, trace) {
cdi.lo = 4 * cdi.q1 - 3 * cdi.q3;
cdi.uo = 4 * cdi.q3 - 3 * cdi.q1;


// lower and upper notches ~95% Confidence Intervals for median
var iqr = cdi.q3 - cdi.q1;
var mci = 1.57 * iqr / Math.sqrt(bvLen);
cdi.ln = cdi.med - mci;
cdi.un = cdi.med + mci;

cd.push(cdi);
}
}
Expand Down
3 changes: 3 additions & 0 deletions src/traces/box/defaults.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ function supplyDefaults(traceIn, traceOut, defaultColor, layout) {
coerce('whiskerwidth');
coerce('boxmean');

var notched = coerce('notched', traceIn.notchwidth !== undefined);
if(notched) coerce('notchwidth');
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we can get away with just a notchwidth similar to how whiskers are set (with whiskerwidth)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you suggesting to remove if(notched)?
Btw. this line was modified after this comment: krassowski#1 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

Are you suggesting to remove if(notched)?

Yes, that's what I'm suggesting. Waiting on a concensus.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think @etpinard was suggesting removing the notched attribute altogether and just defaulting to notchwidth=0 as no notch. I thought about this but would prefer the existing way so that we can pick a nice default notchwidth rather than everyone making up random values. One thing we could do though is make the default for notched be true if traceIn.notchwidth !== undefined. That way if you do want a custom width you don’t also need to provide notched.

Copy link
Contributor

Choose a reason for hiding this comment

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

was suggesting removing the notched attribute altogether and just defaulting to notchwidth=0 as no notch

yes, this is what I meant. Sorry for the confusion.

Copy link
Contributor

Choose a reason for hiding this comment

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

I thought about this but would prefer the existing way so that we can pick a nice default notchwidth rather than everyone making up random values

Good point here. I wonder if we should do the same for whiskers in v2? That is, adding a showwhiskers boolean and making the whiskerswidth default smarter?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if we should do the same for whiskers in v2?

My argument doesn't really apply to whiskers - though I'm not sure I get what you mean by smarter default. If we're happy with the default whiskerwidth staying at 0.5 then people can use 0 (a very non-random number 😄 ) to disable whiskers and any other number for a custom width.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@krassowski in talking with @etpinard offline, we agreed that despite the correspondence with whiskerwidth the Matlab convention makes more sense, to us anyway - so that's 0=no notch, 0.5=meet in the middle, with a default of 0.25.

And re: my comment:

One thing we could do though is make the default for notched be true if traceIn.notchwidth !== undefined.

That's just removing attributes.notched.dflt and changing its coerce line to:
var notched = coerce('notched', traceIn.notchwidth !== undefined);

You'll also want to merge in the latest master so tests pass, then I think we're ready to go!


handlePointsDefaults(traceIn, traceOut, coerce, {prefix: 'box'});
}

Expand Down
24 changes: 20 additions & 4 deletions src/traces/box/plot.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,8 @@ function plotBoxAndWhiskers(sel, axes, trace, t) {
var wdPos = t.wdPos || 0;
var bPosPxOffset = t.bPosPxOffset || 0;
var whiskerWidth = trace.whiskerwidth || 0;
var notched = trace.notched || false;
var nw = notched ? 1 - trace.notchwidth : 1;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we're off by a factor of 2 here - 1 - 2 * trace.notchwidth?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So notchedwidth will accept a float from range (0, 0.5) instead of current (0, 1)?

Copy link
Collaborator

Choose a reason for hiding this comment

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

haha simultaneous comments :) yes.


// to support for one-sided box
var bdPos0;
Expand All @@ -125,6 +127,8 @@ function plotBoxAndWhiskers(sel, axes, trace, t) {
var pos1 = posAxis.c2p(pos + bPos + bdPos1, true) + bPosPxOffset;
var posw0 = posAxis.c2p(pos + bPos - wdPos, true) + bPosPxOffset;
var posw1 = posAxis.c2p(pos + bPos + wdPos, true) + bPosPxOffset;
var posm0 = posAxis.c2p(pos + bPos - bdPos0 * nw, true) + bPosPxOffset;
var posm1 = posAxis.c2p(pos + bPos + bdPos1 * nw, true) + bPosPxOffset;
var q1 = valAxis.c2p(d.q1, true);
var q3 = valAxis.c2p(d.q3, true);
// make sure median isn't identical to either of the
Expand All @@ -135,18 +139,30 @@ function plotBoxAndWhiskers(sel, axes, trace, t) {
);
var lf = valAxis.c2p(trace.boxpoints === false ? d.min : d.lf, true);
var uf = valAxis.c2p(trace.boxpoints === false ? d.max : d.uf, true);
var ln = valAxis.c2p(d.ln, true);
var un = valAxis.c2p(d.un, true);

if(trace.orientation === 'h') {
d3.select(this).attr('d',
'M' + m + ',' + pos0 + 'V' + pos1 + // median line
'M' + q1 + ',' + pos0 + 'V' + pos1 + 'H' + q3 + 'V' + pos0 + 'Z' + // box
'M' + m + ',' + posm0 + 'V' + posm1 + // median line
'M' + q1 + ',' + pos0 + 'V' + pos1 + // left edge
(notched ? 'H' + ln + 'L' + m + ',' + posm1 + 'L' + un + ',' + pos1 : '') + // top notched edge
'H' + q3 + // end of the top edge
'V' + pos0 + // right edge
(notched ? 'H' + un + 'L' + m + ',' + posm0 + 'L' + ln + ',' + pos0 : '') + // bottom notched edge
'Z' + // end of the box
'M' + q1 + ',' + posc + 'H' + lf + 'M' + q3 + ',' + posc + 'H' + uf + // whiskers
((whiskerWidth === 0) ? '' : // whisker caps
'M' + lf + ',' + posw0 + 'V' + posw1 + 'M' + uf + ',' + posw0 + 'V' + posw1));
} else {
d3.select(this).attr('d',
'M' + pos0 + ',' + m + 'H' + pos1 + // median line
'M' + pos0 + ',' + q1 + 'H' + pos1 + 'V' + q3 + 'H' + pos0 + 'Z' + // box
'M' + posm0 + ',' + m + 'H' + posm1 + // median line
'M' + pos0 + ',' + q1 + 'H' + pos1 + // top of the box
(notched ? 'V' + ln + 'L' + posm1 + ',' + m + 'L' + pos1 + ',' + un : '') + // notched right edge
'V' + q3 + // end of the right edge
'H' + pos0 + // bottom of the box
(notched ? 'V' + un + 'L' + posm0 + ',' + m + 'L' + pos0 + ',' + ln : '') + // notched left edge
'Z' + // end of the box
'M' + posc + ',' + q1 + 'V' + lf + 'M' + posc + ',' + q3 + 'V' + uf + // whiskers
((whiskerWidth === 0) ? '' : // whisker caps
'M' + posw0 + ',' + lf + 'H' + posw1 + 'M' + posw0 + ',' + uf + 'H' + posw1));
Expand Down
Binary file added test/image/baselines/box_horz_notched.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.
Binary file added test/image/baselines/box_notched.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.
290 changes: 290 additions & 0 deletions test/image/mocks/box_horz_notched.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,290 @@
{
"data":[
{
"x":[
90,
88,
55,
88,
72,
100,
88,
25,
92,
100,
82,
82,
90,
68,
85,
82,
40,
100,
92,
82,
55,
62,
85,
100,
75,
88,
78,
80,
92,
100,
88,
72,
95,
80,
90,
72,
100,
100,
75,
82,
60,
90,
85,
90,
38,
78,
82,
100,
90,
80,
80,
100,
70,
100,
82,
62,
92,
100,
80,
100,
88,
85
],
"line":{
"color":"#1c9099"
},
"type":"box",
"name":"Notched = True",
"orientation": "h",
"notched":true
},
{
"x":[
70,
65,
85,
75,
72,
75,
90,
88,
85,
80,
92,
85,
85,
75,
72,
80,
42,
80,
95,
90,
62,
65,
65,
82,
68,
48,
57,
95,
70,
100,
80,
95,
78,
80,
80,
85,
90,
100,
52,
85,
72,
70,
45,
75,
85,
95,
65,
70,
85,
70,
85,
35,
90,
95,
95,
65,
62,
48,
60,
85,
85,
90,
70,
68
],
"line":{
"color":"#1c9099"
},
"type":"box",
"name":"Notch width = 0.1",
"notched":true,
"orientation": "h",
"notchwidth":0.1
Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually looks like the mocks and baseline images got out of sync. This one uses 0.2 in both images.
https://6171-45646037-gh.circle-artifacts.com/0/home/ubuntu/plotly.js/build/test_images_diff/diff-box_notched.png
https://6171-45646037-gh.circle-artifacts.com/0/home/ubuntu/plotly.js/build/test_images_diff/diff-box_horz_notched.png

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right. I updated the baselines, hope that all will work this time.

},
{
"x":[
95,
75,
70,
72,
52,
70,
82,
90,
95,
80,
68,
88,
82,
52,
80,
78,
57,
88,
88,
100,
50,
65,
78,
92,
65,
50,
60,
88,
100,
50,
90,
70,
60,
72,
75,
95,
100,
45,
68,
72,
45,
60,
78,
85,
92,
45,
68,
70,
85,
82,
62,
75,
100,
80,
65,
52,
48,
57,
100,
72,
100,
80,
65
],
"line":{
"color":"#1c9099"
},
"type":"box",
"name":"Notch width = 0",
"notched":true,
"orientation": "h",
"notchwidth":0
},
{
"x":[
86,
64,
86.2,
91.1,
86,
85.3,
85.30000000000001,
63.7,
47.400000000000006,
89.2,
91.3,
91.6,
55.099999999999994,
64.1,
76.2,
91.4,
68.9,
60.8,
64.5,
87.7,
21,
97.6,
90.9,
86.6,
100,
33.6,
82.7,
63.599999999999994,
55.5,
80.7,
85,
92.30000000000001,
48.9,
85
],
"line":{
"color":"#1c9099"
},
"type":"box",
"name":"Outside of hinges",
"orientation": "h",
"notched":true
}
],
"layout":{
"showlegend":false,
"xaxis":{
"title":"Grade [%]",
"type":"linear"
},
"title":"Based on Fig 4.4a: Course Grade Distributions",
"yaxis":{
"type":"category"
},
"height":598,
"width":1080,
"margin":{
"l":110
},
"autosize":true
}
}
Loading