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.

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

React finance precursor #2525

Merged
merged 10 commits into from
Apr 5, 2018
8 changes: 7 additions & 1 deletion src/plot_api/plot_api.js
Original file line number Diff line number Diff line change
Expand Up @@ -2429,8 +2429,14 @@ function diffData(gd, oldFullData, newFullData, immutable) {
gd: gd
};


var seenUIDs = {};

for(i = 0; i < oldFullData.length; i++) {
trace = newFullData[i];
trace = newFullData[i]._fullInput;
if(seenUIDs[trace.uid]) continue;
Copy link
Contributor

Choose a reason for hiding this comment

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

similarly here, can we 🔒 this thing?

seenUIDs[trace.uid] = 1;

diffOpts.autoranged = trace.xaxis ? (
Axes.getFromId(gd, trace.xaxis).autorange ||
Axes.getFromId(gd, trace.yaxis).autorange
Expand Down
18 changes: 12 additions & 6 deletions src/plots/plots.js
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ plots.supplyDefaults = function(gd) {
if(_module.cleanData) _module.cleanData(newFullData);
}

if(oldFullData.length === newData.length) {
if(oldFullData.length === newFullData.length) {
Copy link
Contributor

Choose a reason for hiding this comment

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

How hard would it be to 🔒 this fix?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was going to add a Plotly.react test in transform_groupby_test for this and diffing on _fullInput, hence c2b11dc, since that's the obvious place where fullData will have a different length from data after finance is converted. But on second thought perhaps that can of worms should wait for #2508? I'll make a note in that issue if you 👍. The fact that all the existing Plotly.react and other tests pass means these didn't cause problems at least.

Copy link
Contributor

Choose a reason for hiding this comment

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

But on second thought perhaps that can of worms should wait for #2508?

sounds good 👌

for(i = 0; i < newFullData.length; i++) {
relinkPrivateKeys(newFullData[i], oldFullData[i]);
}
Expand Down Expand Up @@ -2364,14 +2364,15 @@ plots.doCalcdata = function(gd, traces) {
// clear stuff that should recomputed in 'regular' loop
if(hasCalcTransform) clearAxesCalc(axList);

// 'regular' loop
for(i = 0; i < fullData.length; i++) {
var cd = [];

function calci(i, isContainer) {
trace = fullData[i];
_module = trace._module;

if(!!_module.isContainer !== isContainer) return;
Copy link
Contributor

Choose a reason for hiding this comment

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

Ha. Nice fix. I wonder if other carpet-dependent blocks (e.g. here) could be simplified (or at least generatlized) using _module.isContainer?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Probably some such blocks could... though I don't see much to do with that carpetDependents loop. It might be nice to move it out of plots/plots and into the carpet module, but probably not in this PR.


var cd = [];

if(trace.visible === true) {
_module = trace._module;

// keep ref of index-to-points map object of the *last* enabled transform,
// this index-to-points map object is required to determine the calcdata indices
Expand Down Expand Up @@ -2406,6 +2407,11 @@ plots.doCalcdata = function(gd, traces) {
calcdata[i] = cd;
}

// 'regular' loop - make sure container traces (eg carpet) calc before
// contained traces (eg contourcarpet)
for(i = 0; i < fullData.length; i++) calci(i, true);
for(i = 0; i < fullData.length; i++) calci(i, false);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Because I moved convertColumnData and setConvert for carpet traces from the supplyDefaults step to the calc step, we need to ensure carpet.calc happens before contourcarpet.calc and scattercarpet.calc. I'm not sure if we're ever going to have another situation like this where one trace essentially lives inside another, but in case we do this seemed like a reasonably generic way to handle it.


Registry.getComponentMethod('fx', 'calc')(gd);
};

Expand Down
2 changes: 1 addition & 1 deletion src/traces/candlestick/defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
}

var len = handleOHLC(traceIn, traceOut, coerce, layout);
if(len === 0) {
if(!len) {
traceOut.visible = false;
return;
}
Expand Down
5 changes: 3 additions & 2 deletions src/traces/candlestick/transform.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@ function makeTrace(traceIn, state, direction) {
xaxis: traceIn.xaxis,
yaxis: traceIn.yaxis,

transforms: helpers.makeTransform(traceIn, state, direction)
transforms: helpers.makeTransform(traceIn, state, direction),
_inputLength: traceIn._inputLength
};

// the rest of below may not have been coerced
Expand Down Expand Up @@ -99,7 +100,7 @@ exports.calcTransform = function calcTransform(gd, trace, opts) {
low = trace.low,
close = trace.close;

var len = open.length,
var len = trace._inputLength,
x = [],
y = [];

Expand Down
3 changes: 3 additions & 0 deletions src/traces/carpet/axis_defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,10 @@ module.exports = function handleAxisDefaults(containerIn, containerOut, options)
handleCalendarDefaults(containerIn, containerOut, 'calendar', options.calendar);
}

// we need some of the other functions setConvert attaches, but for
// path finding, override pixel scaling to simple passthrough (identity)
setConvert(containerOut, options.fullLayout);
containerOut.c2p = Lib.identity;

var dfltColor = coerce('color', options.dfltColor);
// if axis.color was provided, use it for fonts too; otherwise,
Expand Down
36 changes: 25 additions & 11 deletions src/traces/carpet/calc.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,25 +17,37 @@ var calcLabels = require('./calc_labels');
var calcClipPath = require('./calc_clippath');
var clean2dArray = require('../heatmap/clean_2d_array');
var smoothFill2dArray = require('./smooth_fill_2d_array');
var hasColumns = require('./has_columns');
var convertColumnData = require('../heatmap/convert_column_xyz');
var setConvert = require('./set_convert');

module.exports = function calc(gd, trace) {
var xa = Axes.getFromId(gd, trace.xaxis || 'x');
var ya = Axes.getFromId(gd, trace.yaxis || 'y');
var xa = Axes.getFromId(gd, trace.xaxis);
var ya = Axes.getFromId(gd, trace.yaxis);
var aax = trace.aaxis;
var bax = trace.baxis;
var a = trace._a = trace.a;
var b = trace._b = trace.b;

var t = {};
var x;
var x = trace.x;
var y = trace.y;
var cols = [];
if(x && !hasColumns(x)) cols.push('x');
if(y && !hasColumns(y)) cols.push('y');

if(cols.length) {
convertColumnData(trace, aax, bax, 'a', 'b', cols);
}

var a = trace._a = trace._a || trace.a;
var b = trace._b = trace._b || trace.b;
x = trace._x || trace.x;
y = trace._y || trace.y;

var t = {};

if(trace._cheater) {
var avals = aax.cheatertype === 'index' ? a.length : a;
var bvals = bax.cheatertype === 'index' ? b.length : b;
x = cheaterBasis(avals, bvals, trace.cheaterslope);
} else {
x = trace.x;
}

trace._x = x = clean2dArray(x);
Expand All @@ -48,12 +60,14 @@ module.exports = function calc(gd, trace) {
smoothFill2dArray(x, a, b);
smoothFill2dArray(y, a, b);

setConvert(trace);

// create conversion functions that depend on the data
trace.setScale();

// Convert cartesian-space x/y coordinates to screen space pixel coordinates:
t.xp = trace.xp = map2dArray(trace.xp, x, xa.c2p);
t.yp = trace.yp = map2dArray(trace.yp, y, ya.c2p);
t.xp = trace._xp = map2dArray(trace._xp, x, xa.c2p);
t.yp = trace._yp = map2dArray(trace._yp, y, ya.c2p);
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these xp and yp values used anywhere? Here they appear to be reset during the plot step.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ha good call. Nothing fails if you 🔪 these lines, which is good because the calc step had better not generate pixel positions, the axis scaling isn't known yet at that point!
Removed in 0221510

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

wow, then map_2d_array became totally unused -> b0af416


// This is a rather expensive scan. Nothing guarantees monotonicity,
// so we need to scan through all data to get proper ranges:
Expand Down Expand Up @@ -88,7 +102,7 @@ module.exports = function calc(gd, trace) {

// Tabulate points for the four segments that bound the axes so that we can
// map to pixel coordinates in the plot function and create a clip rect:
t.clipsegments = calcClipPath(trace.xctrl, trace.yctrl, aax, bax);
t.clipsegments = calcClipPath(trace._xctrl, trace._yctrl, aax, bax);

t.x = x;
t.y = y;
Expand Down
12 changes: 6 additions & 6 deletions src/traces/carpet/calc_gridlines.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,26 +16,26 @@ module.exports = function calcGridlines(trace, cd, axisLetter, crossAxisLetter)
var eps, bounds, n1, n2, n, value, v;
var j1, v0, v1, d;

var data = trace[axisLetter];
var data = trace['_' + axisLetter];
var axis = trace[axisLetter + 'axis'];

var gridlines = axis._gridlines = [];
var minorgridlines = axis._minorgridlines = [];
var boundarylines = axis._boundarylines = [];

var crossData = trace[crossAxisLetter];
var crossData = trace['_' + crossAxisLetter];
var crossAxis = trace[crossAxisLetter + 'axis'];

if(axis.tickmode === 'array') {
axis.tickvals = data.slice();
}

var xcp = trace.xctrl;
var ycp = trace.yctrl;
var xcp = trace._xctrl;
var ycp = trace._yctrl;
var nea = xcp[0].length;
var neb = xcp.length;
var na = trace.a.length;
var nb = trace.b.length;
var na = trace._a.length;
var nb = trace._b.length;

Axes.prepTicks(axis);

Expand Down
3 changes: 0 additions & 3 deletions src/traces/carpet/defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
var Lib = require('../../lib');
var handleXYDefaults = require('./xy_defaults');
var handleABDefaults = require('./ab_defaults');
var setConvert = require('./set_convert');
var attributes = require('./attributes');
var colorAttrs = require('../../components/color/attributes');

Expand Down Expand Up @@ -49,8 +48,6 @@ module.exports = function supplyDefaults(traceIn, traceOut, dfltColor, fullLayou
// and i goes from 0 to a.length - 1.
var len = handleXYDefaults(traceIn, traceOut, coerce);

setConvert(traceOut);

if(traceOut._cheater) {
coerce('cheaterslope');
}
Expand Down
1 change: 1 addition & 0 deletions src/traces/carpet/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ Carpet.supplyDefaults = require('./defaults');
Carpet.plot = require('./plot');
Carpet.calc = require('./calc');
Carpet.animatable = true;
Carpet.isContainer = true; // so carpet traces get `calc` before other traces

Carpet.moduleType = 'trace';
Carpet.name = 'carpet';
Expand Down
24 changes: 10 additions & 14 deletions src/traces/carpet/set_convert.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@ var createJDerivativeEvaluator = require('./create_j_derivative_evaluator');
* p: screen-space pixel coordinates
*/
module.exports = function setConvert(trace) {
var a = trace.a;
var b = trace.b;
var na = trace.a.length;
var nb = trace.b.length;
var a = trace._a;
var b = trace._b;
var na = a.length;
var nb = b.length;
var aax = trace.aaxis;
var bax = trace.baxis;

Expand Down Expand Up @@ -60,10 +60,6 @@ module.exports = function setConvert(trace) {
return a < amin || a > amax || b < bmin || b > bmax;
};

// XXX: ONLY PASSTHRU. ONLY. No, ONLY.
aax.c2p = function(v) { return v; };
bax.c2p = function(v) { return v; };

trace.setScale = function() {
var x = trace._x;
var y = trace._y;
Expand All @@ -72,18 +68,18 @@ module.exports = function setConvert(trace) {
// an expanded basis of control points. Note in particular that it overwrites the existing
// basis without creating a new array since that would potentially thrash the garbage
// collector.
var result = computeControlPoints(trace.xctrl, trace.yctrl, x, y, aax.smoothing, bax.smoothing);
trace.xctrl = result[0];
trace.yctrl = result[1];
var result = computeControlPoints(trace._xctrl, trace._yctrl, x, y, aax.smoothing, bax.smoothing);
trace._xctrl = result[0];
trace._yctrl = result[1];

// This step is the second step in the process, but it's somewhat simpler. It just unrolls
// some logic since it would be unnecessarily expensive to compute both interpolations
// nearly identically but separately and to include a bunch of linear vs. bicubic logic in
// every single call.
trace.evalxy = createSplineEvaluator([trace.xctrl, trace.yctrl], na, nb, aax.smoothing, bax.smoothing);
trace.evalxy = createSplineEvaluator([trace._xctrl, trace._yctrl], na, nb, aax.smoothing, bax.smoothing);

trace.dxydi = createIDerivativeEvaluator([trace.xctrl, trace.yctrl], aax.smoothing, bax.smoothing);
trace.dxydj = createJDerivativeEvaluator([trace.xctrl, trace.yctrl], aax.smoothing, bax.smoothing);
trace.dxydi = createIDerivativeEvaluator([trace._xctrl, trace._yctrl], aax.smoothing, bax.smoothing);
trace.dxydj = createJDerivativeEvaluator([trace._xctrl, trace._yctrl], aax.smoothing, bax.smoothing);
};

/*
Expand Down
21 changes: 2 additions & 19 deletions src/traces/carpet/xy_defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,28 +9,11 @@

'use strict';

var hasColumns = require('./has_columns');
var convertColumnData = require('../heatmap/convert_column_xyz');

module.exports = function handleXYDefaults(traceIn, traceOut, coerce) {
var cols = [];
var x = coerce('x');

var needsXTransform = x && !hasColumns(x);
if(needsXTransform) cols.push('x');

traceOut._cheater = !x;

var y = coerce('y');

var needsYTransform = y && !hasColumns(y);
if(needsYTransform) cols.push('y');

if(!x && !y) return;

if(cols.length) {
convertColumnData(traceOut, traceOut.aaxis, traceOut.baxis, 'a', 'b', cols);
}
traceOut._cheater = !x;

return true;
return !!x || !!y;
};
2 changes: 1 addition & 1 deletion src/traces/contour/empty_pathinfo.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ module.exports = function emptyPathinfo(contours, plotinfo, cd0) {
var pathinfo = [];
var end = endPlus(contoursFinal);

var carpet = cd0.trace.carpetTrace;
var carpet = cd0.trace._carpetTrace;

var basePathinfo = carpet ? {
// store axes so we can convert to px
Expand Down
12 changes: 7 additions & 5 deletions src/traces/contourcarpet/calc.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ var setContours = require('../contour/set_contours');
// though a few things inside heatmap calc still look for
// contour maps, because the makeBoundArray calls are too entangled
module.exports = function calc(gd, trace) {
var carpet = trace.carpetTrace = lookupCarpet(gd, trace);
var carpet = trace._carpetTrace = lookupCarpet(gd, trace);
if(!carpet || !carpet.visible || carpet.visible === 'legendonly') return;

if(!trace.a || !trace.b) {
Expand Down Expand Up @@ -54,7 +54,7 @@ module.exports = function calc(gd, trace) {
function heatmappishCalc(gd, trace) {
// prepare the raw data
// run makeCalcdata on x and y even for heatmaps, in case of category mappings
var carpet = trace.carpetTrace;
var carpet = trace._carpetTrace;
var aax = carpet.aaxis;
var bax = carpet.baxis;
var a,
Expand All @@ -70,15 +70,17 @@ function heatmappishCalc(gd, trace) {
bax._minDtick = 0;

if(hasColumns(trace)) convertColumnData(trace, aax, bax, 'a', 'b', ['z']);
a = trace._a = trace._a || trace.a;
b = trace._b = trace._b || trace.b;

a = trace.a ? aax.makeCalcdata(trace, 'a') : [];
b = trace.b ? bax.makeCalcdata(trace, 'b') : [];
a = a ? aax.makeCalcdata(trace, '_a') : [];
b = b ? bax.makeCalcdata(trace, '_b') : [];
a0 = trace.a0 || 0;
da = trace.da || 1;
b0 = trace.b0 || 0;
db = trace.db || 1;

z = clean2dArray(trace.z, trace.transpose);
z = trace._z = clean2dArray(trace._z || trace.z, trace.transpose);

trace._emptypoints = findEmpties(z);
trace._interpz = interp2d(z, trace._emptypoints, trace._interpz);
Expand Down
2 changes: 1 addition & 1 deletion src/traces/contourcarpet/plot.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ module.exports = function plot(gd, plotinfo, cdcontours) {
function plotOne(gd, plotinfo, cd) {
var trace = cd[0].trace;

var carpet = trace.carpetTrace = lookupCarpet(gd, trace);
var carpet = trace._carpetTrace = lookupCarpet(gd, trace);
var carpetcd = gd.calcdata[carpet.index][0];

if(!carpet.visible || carpet.visible === 'legendonly') return;
Expand Down
10 changes: 6 additions & 4 deletions src/traces/heatmap/calc.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,12 @@ module.exports = function calc(gd, trace) {
z = binned.z;
}
else {
var zIn = trace.z;
if(hasColumns(trace)) {
convertColumnData(trace, xa, ya, 'x', 'y', ['z']);
x = trace.x;
y = trace.y;
x = trace._x;
y = trace._y;
zIn = trace._z;
} else {
x = trace.x ? xa.makeCalcdata(trace, 'x') : [];
y = trace.y ? ya.makeCalcdata(trace, 'y') : [];
Expand All @@ -72,7 +74,7 @@ module.exports = function calc(gd, trace) {
y0 = trace.y0 || 0;
dy = trace.dy || 1;

z = clean2dArray(trace.z, trace.transpose);
z = clean2dArray(zIn, trace.transpose);

if(isContour || trace.connectgaps) {
trace._emptypoints = findEmpties(z);
Expand Down Expand Up @@ -131,7 +133,7 @@ module.exports = function calc(gd, trace) {
x: xArray,
y: yArray,
z: z,
text: trace.text
text: trace._text || trace.text
};

if(xIn && xIn.length === xArray.length - 1) cd0.xCenter = xIn;
Expand Down
Loading