-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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.
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
add 'width' to box and violin trace #3109
Changes from 11 commits
8f04d13
f76ca97
81cdbe9
5ef2eeb
1c6b26b
2d93a0e
1f9ced8
26f44c4
77b9c9d
243a369
eb5e9da
eea273b
408aacb
0555283
4eb8b44
44e0a88
f8b963b
54aa355
0d4239b
fa3a314
c63780f
c24330d
6e7883d
37786d6
45ae47d
8366c88
ce81377
8885b5e
57cc05a
fe3d7ed
44fa269
f5ba38c
34722d3
d1aed48
c70f7d5
0dea0e9
5e5f9ee
d04c60c
91497e5
657848f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -90,18 +90,33 @@ function setPositionOffset(traceType, gd, boxList, posAxis, pad) { | |
var groupgap = fullLayout[traceType + 'groupgap']; | ||
var padfactor = (1 - gap) * (1 - groupgap) * dPos / fullLayout[numKey]; | ||
|
||
// Find maximum trace width | ||
// we baseline this at dPos | ||
var max_half_width = dPos; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Camel case please 🐫 |
||
for(i = 0; i < boxList.length; i++) { | ||
calcTrace = calcdata[boxList[i]]; | ||
|
||
if(calcTrace[0].trace.width) { | ||
if(calcTrace[0].trace.width / 2 > max_half_width) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm. What if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It actually does work. See this line where the width is set. These lines are affecting the axis autoscale of the plotted points. But if a violin has trace.width = 0.00001 (much less than the min-diff as you mention) then that trace will shrink to the whatever the width is set to but the "camera" will not zoom in anymore. The view of all the traces is always calibrated to the violin/box with the maximum width set, which doesn't loose any desired layout that you could want with violins or boxes, if you decide to use width at all. |
||
max_half_width = calcTrace[0].trace.width / 2; | ||
} | ||
} | ||
} | ||
alexcjohnson marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// autoscale the x axis - including space for points if they're off the side | ||
// TODO: this will overdo it if the outermost boxes don't have | ||
// their points as far out as the other boxes | ||
var extremes = Axes.findExtremes(posAxis, boxdv.vals, { | ||
vpadminus: dPos + pad[0] * padfactor, | ||
vpadplus: dPos + pad[1] * padfactor | ||
vpadminus: max_half_width + pad[0] * padfactor, | ||
vpadplus: max_half_width + pad[1] * padfactor | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah good catch - unfortunately the It looks to me as though we could pretty easily fix this by calculating var thisDPos = calcTrace[0].t.dPos = (calcTrace[0].trace.width / 2) || dPos;
var positions = calcTrace.map(function(di) { return di.pos });
var extremes = Axes.findExtremes(posAxis, positions, {
vpadminus: thisDPos + pad[0] * padfactor,
vpadminus: thisDPos + pad[1] * padfactor
});
calcTrace[0].trace._extremes[posAxis._id] = extremes; That wouldn't address the original There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not that much improvement, but there is some. Notice the vertical distance between the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, right - because we don't take There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
}); | ||
|
||
for(i = 0; i < boxList.length; i++) { | ||
calcTrace = calcdata[boxList[i]]; | ||
// set the width of all boxes | ||
calcTrace[0].t.dPos = dPos; | ||
// override dPos with trace.width if present | ||
calcTrace[0].t.dPos = (calcTrace[0].trace.width / 2) || dPos; | ||
|
||
// link extremes to all boxes | ||
calcTrace[0].trace._extremes[posAxis._id] = extremes; | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,64 @@ | ||
{ | ||
"data": [{ | ||
"type": "violin", | ||
"width": 0.315, | ||
"x": [0, 5, 7, 8], | ||
"points": "none", | ||
"side": "positive", | ||
"box": { | ||
"visible": false | ||
}, | ||
"boxpoints": false, | ||
"line": { | ||
"color": "black" | ||
}, | ||
"fillcolor": "#8dd3c7", | ||
"opacity": 0.6, | ||
"meanline": { | ||
"visible": false | ||
}, | ||
"y0": 0.0 | ||
}, { | ||
"type": "violin", | ||
"x": [0, 5, 7, 8], | ||
"points": "none", | ||
"side": "positive", | ||
"box": { | ||
"visible": false | ||
}, | ||
"boxpoints": false, | ||
"line": { | ||
"color": "black" | ||
}, | ||
"fillcolor": "#d3c78d", | ||
"opacity": 0.6, | ||
"meanline": { | ||
"visible": false | ||
}, | ||
"y0": 0.1 | ||
}, { | ||
"type": "box", | ||
"width": 0.5421, | ||
"x": [0, 5, 7, 8], | ||
"points": "none", | ||
"side": "positive", | ||
"box": { | ||
"visible": false | ||
}, | ||
"boxpoints": false, | ||
"line": { | ||
"color": "black" | ||
}, | ||
"fillcolor": "#c78dd3", | ||
"opacity": 0.6, | ||
"meanline": { | ||
"visible": false | ||
}, | ||
"y0": 0.2 | ||
}], | ||
"layout": { | ||
"title": "Joyplot - Violin with multiple widths", | ||
"xaxis": {"zeroline": false}, | ||
"violingap": 0 | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
⚡ that
,
Not sure why our linting didn't pick this up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added this as I placed the width property at the end of this list the first time around. I then moved to the middle to a more appropriate spot but forgot to ⚡️ the
,