Merge pull request #1655 from plotly/pr-1619-ohlc-equal-open-close

OHLC equal open close cases fix

Author: alexcjohnson

src/traces/candlestick/transform.js

@@ -9,6 +9,8 @@
 
 'use strict';
 
+var isNumeric = require('fast-isnumeric');
+
 var Lib = require('../../lib');
 var helpers = require('../ohlc/helpers');
 
@@ -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]);
         }

src/traces/ohlc/helpers.js

@@ -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.
@@ -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);
+    }
+
+    return direction === 'increasing' ? isIncreasing : isDecreasing;
+}
 
 exports.addRangeSlider = function(data, layout) {
     var hasOneVisibleTrace = false;

src/traces/ohlc/transform.js

@@ -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');
@@ -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]);

test/image/baselines/candlestick_double-y-axis.png

@@ -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]);
 
@@ -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() {