Skip to content

scattergl errorbars fixes #2620

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 6 commits into from
May 22, 2018
Merged

scattergl errorbars fixes #2620

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2f4786d
fix errorbar when coord input arrays differ from their calc arrays
etpinard May 8, 2018
8837b2e
Merge branch 'master' into scattergl-errors
etpinard May 22, 2018
82290a2
replace calcFromTrace wrapper with more direct computeError routine
etpinard May 22, 2018
ded2f56
add makeComputeError to errorbars module, call it w/ Registry
etpinard May 22, 2018
c1b1730
add percent error bar to gl2d log error bar mock
etpinard May 22, 2018
7e77bb4
:palm_tree: convertErrorBarPositions
etpinard May 22, 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
16 changes: 8 additions & 8 deletions src/traces/scattergl/convert.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -360,10 +360,11 @@ function convertLinePositions(gd, trace, positions) {
};
}

function convertErrorBarPositions(gd, trace, positions) {
function convertErrorBarPositions(gd, trace, pos, x, y) {
var calcFromTrace = Registry.getComponentMethod('errorbars', 'calcFromTrace');
var vals = calcFromTrace(trace, gd._fullLayout);
var count = positions.length / 2;
var _trace = Lib.extendFlat({}, trace, {x: x, y: y});
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems weird to me - putting arrays of calcdata values into a fake trace. I don't know if category or date data would make any sense in this context but if we were to support them this would break.

This is the only caller of calcFromTrace, right? Can we just pass in the x and y calc'ed arrays separately from the trace?

Copy link
Contributor Author

@etpinard etpinard May 22, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Good call. Using calcFromTrace made things very dry 🌴 but added a lot of overhead and many (many) object creations. So yeah good riddance 🔪 -> 82290a2

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah nice - that's more of a refactor than I was expecting but you're right, much better. 🌴 before, but as well as the 🐎 overhead, the scaffolding meant it didn't even really result in shorter code.

var vals = calcFromTrace(_trace, gd._fullLayout);
var count = pos.length / 2;
var out = {};

function put(axLetter) {
Expand All @@ -373,25 +374,24 @@ function convertErrorBarPositions(gd, trace, positions) {
var eOffset = {x: [0, 1, 2, 3], y: [2, 3, 0, 1]}[axLetter];

for(var i = 0, p = 0; i < count; i++, p += 4) {
errors[p + eOffset[0]] = positions[i * 2 + pOffset] - ax.d2l(vals[i][axLetter + 's']) || 0;
errors[p + eOffset[1]] = ax.d2l(vals[i][axLetter + 'h']) - positions[i * 2 + pOffset] || 0;
errors[p + eOffset[0]] = pos[i * 2 + pOffset] - ax.c2l(vals[i][axLetter + 's']) || 0;
errors[p + eOffset[1]] = ax.c2l(vals[i][axLetter + 'h']) - pos[i * 2 + pOffset] || 0;
errors[p + eOffset[2]] = 0;
errors[p + eOffset[3]] = 0;
}

return errors;
}


if(trace.error_x && trace.error_x.visible) {
out.x = {
positions: positions,
positions: pos,
errors: put('x')
};
}
if(trace.error_y && trace.error_y.visible) {
out.y = {
positions: positions,
positions: pos,
errors: put('y')
};
}
Expand Down
6 changes: 3 additions & 3 deletions src/traces/scattergl/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ function calc(gd, trace) {

// create scene options and scene
calcColorscales(trace);
var opts = sceneOptions(gd, subplot, trace, positions);
var opts = sceneOptions(gd, subplot, trace, positions, x, y);
var scene = sceneUpdate(gd, subplot);

// Re-use SVG scatter axis expansion routine except
Expand Down Expand Up @@ -133,7 +133,7 @@ function calc(gd, trace) {
}

// create scene options
function sceneOptions(gd, subplot, trace, positions) {
function sceneOptions(gd, subplot, trace, positions, x, y) {
var opts = convertStyle(gd, trace);

if(opts.marker) {
Expand All @@ -148,7 +148,7 @@ function sceneOptions(gd, subplot, trace, positions) {
}

if(opts.errorX || opts.errorY) {
var errors = convertErrorBarPositions(gd, trace, positions);
var errors = convertErrorBarPositions(gd, trace, positions, x, y);

if(opts.errorX) {
Lib.extendFlat(opts.errorX, errors.x);
Expand Down
Binary file modified test/image/baselines/gl2d_error_bars.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.
23 changes: 23 additions & 0 deletions test/image/mocks/gl2d_error_bars.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,15 @@
{
"data": [
{
"x": [6],
"y": ["2"],
"error_y": {
"type": "data",
"array": [2],
"arrayminus": [4]
},
"type": "scattergl"
},
{
"x": [
0,
Expand Down Expand Up @@ -92,6 +102,19 @@
"value": 10
},
"type": "scattergl"
},
{
"y": [
3,
3,
6,
5
],
"error_x": {
"type": "constant",
"value": 0.1
},
"type": "scattergl"
}
]
}