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

OHLC equal open close cases fix #1655

Merged
merged 8 commits into from
Jun 8, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 3 additions & 1 deletion src/traces/candlestick/transform.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@

'use strict';

var isNumeric = require('fast-isnumeric');

var Lib = require('../../lib');
var helpers = require('../ohlc/helpers');

Expand Down Expand Up @@ -115,7 +117,7 @@ exports.calcTransform = function calcTransform(gd, trace, opts) {
};

for(var i = 0; i < len; i++) {
if(filterFn(open[i], close[i])) {
if(filterFn(open[i], close[i]) && isNumeric(high[i]) && isNumeric(low[i])) {
appendX(i);
appendY(open[i], high[i], low[i], close[i]);
}
Expand Down
38 changes: 32 additions & 6 deletions src/traces/ohlc/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@

'use strict';

var isNumeric = require('fast-isnumeric');

var Lib = require('../../lib');

// This routine gets called during the trace supply-defaults step.
Expand Down Expand Up @@ -95,14 +97,38 @@ exports.makeTransform = function(traceIn, state, direction) {
};

exports.getFilterFn = function(direction) {
switch(direction) {
case 'increasing':
return function(o, c) { return o <= c; };
return new _getFilterFn(direction);
};

case 'decreasing':
return function(o, c) { return o > c; };
function _getFilterFn(direction) {
// we're optimists - before we have any changing data, assume increasing
var isPrevIncreasing = true;
var cPrev = null;

function _isIncreasing(o, c) {
if(o === c) {
if(c > cPrev) {
isPrevIncreasing = true; // increasing
} else if(c < cPrev) {
isPrevIncreasing = false; // decreasing
}
// else isPrevIncreasing is not changed
}
else isPrevIncreasing = (o < c);
cPrev = c;
return isPrevIncreasing;
}
};

function isIncreasing(o, c) {
return isNumeric(o) && isNumeric(c) && _isIncreasing(+o, +c);
}

function isDecreasing(o, c) {
return isNumeric(o) && isNumeric(c) && !_isIncreasing(+o, +c);
Copy link
Contributor Author

@etpinard etpinard Jun 8, 2017

Choose a reason for hiding this comment

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

that !_isIncreasing statement is so brilliant ✨

}

return direction === 'increasing' ? isIncreasing : isDecreasing;
}

exports.addRangeSlider = function(data, layout) {
var hasOneVisibleTrace = false;
Expand Down
4 changes: 3 additions & 1 deletion src/traces/ohlc/transform.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@

'use strict';

var isNumeric = require('fast-isnumeric');

var Lib = require('../../lib');
var helpers = require('./helpers');
var Axes = require('../../plots/cartesian/axes');
Expand Down Expand Up @@ -195,7 +197,7 @@ exports.calcTransform = function calcTransform(gd, trace, opts) {
};

for(var i = 0; i < len; i++) {
if(filterFn(open[i], close[i])) {
if(filterFn(open[i], close[i]) && isNumeric(high[i]) && isNumeric(low[i])) {
appendX(i);
appendY(open[i], high[i], low[i], close[i]);
appendText(i, open[i], high[i], low[i], close[i]);
Expand Down
Binary file modified test/image/baselines/candlestick_double-y-axis.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/candlestick_rangeslider_thai.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
53 changes: 53 additions & 0 deletions test/jasmine/tests/finance_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -395,14 +395,28 @@ describe('finance charts calc transforms:', function() {
return gd.calcdata.map(calcDatatoTrace);
}

// add some points that shouldn't make it into calcdata because
// one of o, h, l, c is not numeric
function addJunk(trace) {
// x filtering happens in other ways
if(trace.x) trace.x.push(1, 1, 1, 1);

trace.open.push('', 1, 1, 1);
trace.high.push(1, null, 1, 1);
trace.low.push(1, 1, [1], 1);
trace.close.push(1, 1, 1, 'close');
}

it('should fill when *x* is not present', function() {
var trace0 = Lib.extendDeep({}, mock0, {
type: 'ohlc',
});
addJunk(trace0);

var trace1 = Lib.extendDeep({}, mock0, {
type: 'candlestick',
});
addJunk(trace1);

var out = _calc([trace0, trace1]);

Expand Down Expand Up @@ -704,6 +718,45 @@ describe('finance charts calc transforms:', function() {
expect(out[1].x).toEqual([]);
expect(out[3].x).toEqual([]);
});

it('should handle cases where \'open\' and \'close\' entries are equal', function() {
var out = _calc([{
type: 'ohlc',
open: [0, 1, 0, 2, 1, 1, 2, 2],
high: [3, 3, 3, 3, 3, 3, 3, 3],
low: [-1, -1, -1, -1, -1, -1, -1, -1],
close: [0, 2, 0, 1, 1, 1, 2, 2],
tickwidth: 0
}, {
type: 'candlestick',
open: [0, 2, 0, 1],
high: [3, 3, 3, 3],
low: [-1, -1, -1, -1],
close: [0, 1, 0, 2]
}]);

expect(out[0].x).toEqual([
0, 0, 0, 0, 0, 0, null,
1, 1, 1, 1, 1, 1, null,
6, 6, 6, 6, 6, 6, null,
7, 7, 7, 7, 7, 7, null
]);
expect(out[1].x).toEqual([
2, 2, 2, 2, 2, 2, null,
3, 3, 3, 3, 3, 3, null,
4, 4, 4, 4, 4, 4, null,
5, 5, 5, 5, 5, 5, null
]);

expect(out[2].x).toEqual([
0, 0, 0, 0, 0, 0,
3, 3, 3, 3, 3, 3
]);
expect(out[3].x).toEqual([
1, 1, 1, 1, 1, 1,
2, 2, 2, 2, 2, 2
]);
});
});

describe('finance charts updates:', function() {
Expand Down