redefine what 'r' means for category axes & add cleanPos axis method

etpinard · etpinard · commit b67657fdf3e6 · 2017-05-30T17:52:50.000-04:00
- DO NOT convert category position value to indices (the old 'r')
  in Axes.coercePosition - as this relied on ax._categories which
  isn't defined during Plots.supplyDefaults.
- modify `type: 'category'` conversions functions accordingly (a bit hacky)
- update annotation/click.js to reflect change in 'r' definition
- use ax.cleanPos to clean position value per axis type in coercePosition
diff --git a/src/components/annotations/click.js b/src/components/annotations/click.js
@@ -83,18 +83,22 @@ function getToggleSets(gd, hoverData) {
         explicitOffSet = [],
         hoverLen = (hoverData || []).length;
 
-    var i, j, anni, showMode, pointj, toggleType;
+    var i, j, anni, showMode, pointj, xa, ya, toggleType;
 
     for(i = 0; i < annotations.length; i++) {
         anni = annotations[i];
         showMode = anni.clicktoshow;
+
         if(showMode) {
             for(j = 0; j < hoverLen; j++) {
                 pointj = hoverData[j];
-                if(pointj.xaxis._id === anni.xref &&
-                    pointj.yaxis._id === anni.yref &&
-                    pointj.xaxis.d2r(pointj.x) === anni._xclick &&
-                    pointj.yaxis.d2r(pointj.y) === anni._yclick
+                xa = pointj.xaxis;
+                ya = pointj.yaxis;
+
+                if(xa._id === anni.xref &&
+                    ya._id === anni.yref &&
+                    xa.d2r(pointj.x) === clickData2r(anni._xclick, xa) &&
+                    ya.d2r(pointj.y) === clickData2r(anni._yclick, ya)
                 ) {
                     // match! toggle this annotation
                     // regardless of its clicktoshow mode
@@ -121,3 +125,8 @@ function getToggleSets(gd, hoverData) {
 
     return {on: onSet, off: offSet, explicitOff: explicitOffSet};
 }
+
+// to handle log axes until v2
+function clickData2r(d, ax) {
+    return ax.type === 'log' ? ax.l2r(d) : ax.d2r(d);
+}
diff --git a/src/lib/index.js b/src/lib/index.js
@@ -10,6 +10,11 @@
 'use strict';
 
 var d3 = require('d3');
+var isNumeric = require('fast-isnumeric');
+
+var numConstants = require('../constants/numerical');
+var FP_SAFE = numConstants.FP_SAFE;
+var BADNUM = numConstants.BADNUM;
 
 var lib = module.exports = {};
 
@@ -87,6 +92,13 @@ lib.pushUnique = require('./push_unique');
 
 lib.cleanNumber = require('./clean_number');
 
+lib.num = function num(v) {
+    if(!isNumeric(v)) return BADNUM;
+    v = Number(v);
+    if(v < -FP_SAFE || v > FP_SAFE) return BADNUM;
+    return isNumeric(v) ? Number(v) : BADNUM;
+};
+
 lib.noop = require('./noop');
 lib.identity = require('./identity');
 
diff --git a/src/plots/cartesian/axes.js b/src/plots/cartesian/axes.js
@@ -27,7 +27,6 @@ var ONEDAY = constants.ONEDAY;
 var ONEHOUR = constants.ONEHOUR;
 var ONEMIN = constants.ONEMIN;
 var ONESEC = constants.ONESEC;
-var BADNUM = constants.BADNUM;
 
 var axes = module.exports = {};
 
@@ -100,33 +99,20 @@ axes.coerceRef = function(containerIn, containerOut, gd, attr, dflt, extraOption
  * - for other types: coerce them to numbers
  */
 axes.coercePosition = function(containerOut, gd, coerce, axRef, attr, dflt) {
-    var pos,
-        newPos;
+    var ax, pos;
 
     if(axRef === 'paper' || axRef === 'pixel') {
+        ax = { cleanPos: Lib.num };
         pos = coerce(attr, dflt);
-    }
-    else {
-        var ax = axes.getFromId(gd, axRef);
-
+    } else {
+        ax = axes.getFromId(gd, axRef);
         dflt = ax.fraction2r(dflt);
         pos = coerce(attr, dflt);
-
-        if(ax.type === 'category') {
-            // if position is given as a category name, convert it to a number
-            if(typeof pos === 'string' && (ax._categories || []).length) {
-                newPos = ax._categories.indexOf(pos);
-                containerOut[attr] = (newPos === -1) ? dflt : newPos;
-                return;
-            }
-        }
-        else if(ax.type === 'date') {
-            containerOut[attr] = Lib.cleanDate(pos, BADNUM, ax.calendar);
-            return;
-        }
     }
-    // finally make sure we have a number (unless date type already returned a string)
-    containerOut[attr] = isNumeric(pos) ? Number(pos) : dflt;
+
+    containerOut[attr] = ax.cleanPos(pos);
+};
+
 };
 
 axes.getDataToCoordFunc = function(gd, trace, target, targetArray) {
diff --git a/src/plots/cartesian/set_convert.js b/src/plots/cartesian/set_convert.js
@@ -16,6 +16,7 @@ var Lib = require('../../lib');
 var cleanNumber = Lib.cleanNumber;
 var ms2DateTime = Lib.ms2DateTime;
 var dateTime2ms = Lib.dateTime2ms;
+var num = Lib.num;
 
 var numConstants = require('../../constants/numerical');
 var FP_SAFE = numConstants.FP_SAFE;
@@ -28,13 +29,6 @@ function fromLog(v) {
     return Math.pow(10, v);
 }
 
-function num(v) {
-    if(!isNumeric(v)) return BADNUM;
-    v = Number(v);
-    if(v < -FP_SAFE || v > FP_SAFE) return BADNUM;
-    return isNumeric(v) ? Number(v) : BADNUM;
-}
-
 /**
  * Define the conversion functions for an axis data is used in 5 ways:
  *
@@ -152,7 +146,7 @@ module.exports = function setConvert(ax, fullLayout) {
             if(index !== undefined) return index;
         }
 
-        if(typeof v === 'number') { return v; }
+        if(isNumeric(v)) return +v;
     }
 
     function l2p(v) {
@@ -186,6 +180,8 @@ module.exports = function setConvert(ax, fullLayout) {
 
         ax.d2p = ax.r2p = function(v) { return ax.l2p(cleanNumber(v)); };
         ax.p2d = ax.p2r = p2l;
+
+        ax.cleanPos = num;
     }
     else if(ax.type === 'log') {
         // d and c are data vals, r and l are logged (but d and r need cleaning)
@@ -203,6 +199,8 @@ module.exports = function setConvert(ax, fullLayout) {
 
         ax.r2p = function(v) { return ax.l2p(cleanNumber(v)); };
         ax.p2r = p2l;
+
+        ax.cleanPos = num;
     }
     else if(ax.type === 'date') {
         // r and d are date strings, l and c are ms
@@ -222,24 +220,31 @@ module.exports = function setConvert(ax, fullLayout) {
 
         ax.d2p = ax.r2p = function(v, _, calendar) { return ax.l2p(dt2ms(v, 0, calendar)); };
         ax.p2d = ax.p2r = function(px, r, calendar) { return ms2dt(p2l(px), r, calendar); };
+
+        ax.cleanPos = function(v) { return Lib.cleanDate(v, BADNUM, ax.calendar); };
     }
     else if(ax.type === 'category') {
-        // d is categories; r, c, and l are indices
-        // TODO: should r accept category names too?
-        // ie r2c and r2l would be getCategoryIndex (and r2p would change)
+        // d is categories (string)
+        // c and l are indices (numbers)
+        // r is categories or numbers
 
-        ax.d2r = ax.d2c = ax.d2l = setCategoryIndex;
+        ax.d2c = ax.d2l = setCategoryIndex;
         ax.r2d = ax.c2d = ax.l2d = getCategoryName;
 
-        // special d2l variant that won't add categories
-        ax.d2l_noadd = getCategoryIndex;
+        ax.d2r = ax.d2l_noadd = getCategoryIndex;
 
-        ax.r2l = ax.l2r = ax.r2c = ax.c2r = num;
+        ax.l2r = ax.r2c = ax.c2r = num;
+        ax.r2l = getCategoryIndex;
 
         ax.d2p = function(v) { return ax.l2p(getCategoryIndex(v)); };
         ax.p2d = function(px) { return getCategoryName(p2l(px)); };
-        ax.r2p = ax.l2p;
+        ax.r2p = ax.d2p;
         ax.p2r = p2l;
+
+        ax.cleanPos = function(v) {
+            if(typeof v === 'string' && v !== '') return v;
+            return num(v);
+        };
     }
 
     // find the range value at the specified (linear) fraction of the axis
diff --git a/src/traces/heatmap/calc.js b/src/traces/heatmap/calc.js
@@ -57,10 +57,15 @@ module.exports = function calc(gd, trace) {
         z = binned.z;
     }
     else {
-        if(hasColumns(trace)) convertColumnData(trace, xa, ya, 'x', 'y', ['z']);
+        if(hasColumns(trace)) {
+            convertColumnData(trace, xa, ya, 'x', 'y', ['z']);
+            x = trace.x;
+            y = trace.y;
+        } else {
+            x = trace.x ? xa.makeCalcdata(trace, 'x') : [];
+            y = trace.y ? ya.makeCalcdata(trace, 'y') : [];
+        }
 
-        x = trace.x ? xa.makeCalcdata(trace, 'x') : [];
-        y = trace.y ? ya.makeCalcdata(trace, 'y') : [];
         x0 = trace.x0 || 0;
         dx = trace.dx || 1;
         y0 = trace.y0 || 0;
diff --git a/test/jasmine/tests/annotations_test.js b/test/jasmine/tests/annotations_test.js
@@ -98,39 +98,16 @@ describe('Test annotations', function() {
             expect(layoutOut.annotations[0].ax).toEqual('2004-07-01');
         });
 
-        it('should convert ax/ay category coordinates to linear coords', function() {
             var layoutIn = {
                 annotations: [{
-                    showarrow: true,
-                    axref: 'x',
-                    ayref: 'y',
-                    x: 'c',
-                    ax: 1,
-                    y: 'A',
-                    ay: 3
                 }]
             };
 
             var layoutOut = {
-                xaxis: {
-                    type: 'category',
-                    _categories: ['a', 'b', 'c'],
-                    range: [-0.5, 2.5] },
-                yaxis: {
-                    type: 'category',
-                    _categories: ['A', 'B', 'C'],
-                    range: [-0.5, 3]
-                }
             };
-            Axes.setConvert(layoutOut.xaxis);
-            Axes.setConvert(layoutOut.yaxis);
 
             _supply(layoutIn, layoutOut);
 
-            expect(layoutOut.annotations[0].x).toEqual(2);
-            expect(layoutOut.annotations[0].ax).toEqual(1);
-            expect(layoutOut.annotations[0].y).toEqual(0);
-            expect(layoutOut.annotations[0].ay).toEqual(3);
         });
     });
 });
