From f1d5ee3e25241c0cbc194d5f300f2b025f92f777 Mon Sep 17 00:00:00 2001 From: Klaus Weidner Date: Tue, 1 Mar 2016 13:57:05 -0800 Subject: [PATCH] Fix yRangePad for logscale graphs, add tests. Also includes a minor refactor to consolidate duplicated code for toDataCoord calculations on logscale axes. Fixes issue #661. --- auto_tests/tests/axis_labels.js | 27 ++++++++ auto_tests/tests/range_tests.js | 2 +- auto_tests/tests/to_dom_coords.js | 14 ++++ src/dygraph-utils.js | 33 +++++++++ src/dygraph.js | 111 +++++++++--------------------- 5 files changed, 108 insertions(+), 79 deletions(-) diff --git a/auto_tests/tests/axis_labels.js b/auto_tests/tests/axis_labels.js index 0af808522..c390392f2 100644 --- a/auto_tests/tests/axis_labels.js +++ b/auto_tests/tests/axis_labels.js @@ -687,6 +687,33 @@ it('testLogScale', function() { assert.deepEqual(['0','200','400','600','800','1000'], Util.getYLabels()); }); +/** + * Verify that log scale axis range works with yRangePad. + * + * This is a regression test for https://github.com/danvk/dygraphs/issues/661 . + */ +it('testLogScalePad', function() { + var g = new Dygraph("graph", + [[0, 1e-5], [1, 0.25], [2, 1], [3, 3], [4, 10]], { + width: 250, + height: 130, + logscale: true, + yRangePad: 30, + axes: {y: {valueRange: [1, 10]}}, + labels: ['X', 'Y'] + }); + var nonEmptyLabels = Util.getYLabels().filter(function(x) { return x.length > 0; }); + assert.deepEqual(['1', '7', '30'], nonEmptyLabels); + + g.updateOptions({ yRangePad: 10, axes: {y: {valueRange: [0.25005, 3]}} }); + nonEmptyLabels = Util.getYLabels().filter(function(x) { return x.length > 0; }); + assert.deepEqual(['0.4', '1', '3'], nonEmptyLabels); + + g.updateOptions({ axes: {y: {valueRange: [0.01, 3]}} }); + nonEmptyLabels = Util.getYLabels().filter(function(x) { return x.length > 0; }); + assert.deepEqual(['0.01','0.1','0.7','5'], nonEmptyLabels); +}); + /** * Verify that include zero range is properly specified. */ diff --git a/auto_tests/tests/range_tests.js b/auto_tests/tests/range_tests.js index ff1018e15..19d429478 100644 --- a/auto_tests/tests/range_tests.js +++ b/auto_tests/tests/range_tests.js @@ -342,7 +342,7 @@ it('testLogscalePad', function() { yRangePad: 30 }, [[-10, 10], [10, 10], [30, 1000]], - [-10, 30], [5.01691, 1993.25801]); + [-10, 30], [5.623, 1778.279]); }); /** diff --git a/auto_tests/tests/to_dom_coords.js b/auto_tests/tests/to_dom_coords.js index 454050115..e5bda3b19 100644 --- a/auto_tests/tests/to_dom_coords.js +++ b/auto_tests/tests/to_dom_coords.js @@ -225,6 +225,20 @@ it('testChartLogarithmic_YAxis', function() { assert.deepEqual([400, 0], g.toDomCoords(10, 4)); assert.deepEqual([400, 400], g.toDomCoords(10, 1)); assert.deepEqual([400, 200], g.toDomCoords(10, 2)); + + // Verify that the margins are adjusted appropriately for yRangePad. + g.updateOptions({yRangePad: 40}); + assertDeepCloseTo([0, 4], g.toDataCoords(0, 40), epsilon); + assertDeepCloseTo([0, 1], g.toDataCoords(0, 360), epsilon); + assertDeepCloseTo([10, 4], g.toDataCoords(400, 40), epsilon); + assertDeepCloseTo([10, 1], g.toDataCoords(400, 360), epsilon); + assertDeepCloseTo([10, 2], g.toDataCoords(400, 200), epsilon); + + assertDeepCloseTo([0, 40], g.toDomCoords(0, 4), epsilon); + assertDeepCloseTo([0, 360], g.toDomCoords(0, 1), epsilon); + assertDeepCloseTo([400, 40], g.toDomCoords(10, 4), epsilon); + assertDeepCloseTo([400, 360], g.toDomCoords(10, 1), epsilon); + assertDeepCloseTo([400, 200], g.toDomCoords(10, 2), epsilon); }); it('testChartLogarithmic_XAxis', function() { diff --git a/src/dygraph-utils.js b/src/dygraph-utils.js index c2ed9bb75..3a3e637b8 100644 --- a/src/dygraph-utils.js +++ b/src/dygraph-utils.js @@ -28,6 +28,39 @@ export var log10 = function(x) { return Math.log(x) / LN_TEN; }; +/** + * @private + * @param {number} r0 + * @param {number} r1 + * @param {number} pct + * @return {number} + */ +export var logRangeFraction = function(r0, r1, pct) { + // Computing the inverse of toPercentXCoord. The function was arrived at with + // the following steps: + // + // Original calcuation: + // pct = (log(x) - log(xRange[0])) / (log(xRange[1]) - log(xRange[0]))); + // + // Multiply both sides by the right-side demoninator. + // pct * (log(xRange[1] - log(xRange[0]))) = log(x) - log(xRange[0]) + // + // add log(xRange[0]) to both sides + // log(xRange[0]) + (pct * (log(xRange[1]) - log(xRange[0])) = log(x); + // + // Swap both sides of the equation, + // log(x) = log(xRange[0]) + (pct * (log(xRange[1]) - log(xRange[0])) + // + // Use both sides as the exponent in 10^exp and we're done. + // x = 10 ^ (log(xRange[0]) + (pct * (log(xRange[1]) - log(xRange[0]))) + + var logr0 = log10(r0); + var logr1 = log10(r1); + var exponent = logr0 + (pct * (logr1 - logr0)); + var value = Math.pow(LOG_SCALE, exponent); + return value; +}; + /** A dotted line stroke pattern. */ export var DOTTED_LINE = [2, 2]; /** A dashed line stroke pattern. */ diff --git a/src/dygraph.js b/src/dygraph.js index 41e5cc15a..e5053ba15 100644 --- a/src/dygraph.js +++ b/src/dygraph.js @@ -632,32 +632,8 @@ Dygraph.prototype.toDataXCoord = function(x) { if (!this.attributes_.getForAxis("logscale", 'x')) { return xRange[0] + (x - area.x) / area.w * (xRange[1] - xRange[0]); } else { - // TODO: remove duplicate code? - // Computing the inverse of toDomCoord. var pct = (x - area.x) / area.w; - - // Computing the inverse of toPercentXCoord. The function was arrived at with - // the following steps: - // - // Original calcuation: - // pct = (log(x) - log(xRange[0])) / (log(xRange[1]) - log(xRange[0]))); - // - // Multiply both sides by the right-side demoninator. - // pct * (log(xRange[1] - log(xRange[0]))) = log(x) - log(xRange[0]) - // - // add log(xRange[0]) to both sides - // log(xRange[0]) + (pct * (log(xRange[1]) - log(xRange[0])) = log(x); - // - // Swap both sides of the equation, - // log(x) = log(xRange[0]) + (pct * (log(xRange[1]) - log(xRange[0])) - // - // Use both sides as the exponent in 10^exp and we're done. - // x = 10 ^ (log(xRange[0]) + (pct * (log(xRange[1]) - log(xRange[0]))) - var logr0 = utils.log10(xRange[0]); - var logr1 = utils.log10(xRange[1]); - var exponent = logr0 + (pct * (logr1 - logr0)); - var value = Math.pow(utils.LOG_SCALE, exponent); - return value; + return utils.logRangeFraction(xRange[0], xRange[1], pct); } }; @@ -681,32 +657,8 @@ Dygraph.prototype.toDataYCoord = function(y, axis) { } else { // Computing the inverse of toDomCoord. var pct = (y - area.y) / area.h; - - // Computing the inverse of toPercentYCoord. The function was arrived at with - // the following steps: - // - // Original calcuation: - // pct = (log(yRange[1]) - log(y)) / (log(yRange[1]) - log(yRange[0])); - // - // Multiply both sides by the right-side demoninator. - // pct * (log(yRange[1]) - log(yRange[0])) = log(yRange[1]) - log(y); - // - // subtract log(yRange[1]) from both sides. - // (pct * (log(yRange[1]) - log(yRange[0]))) - log(yRange[1]) = -log(y); - // - // and multiply both sides by -1. - // log(yRange[1]) - (pct * (logr1 - log(yRange[0])) = log(y); - // - // Swap both sides of the equation, - // log(y) = log(yRange[1]) - (pct * (log(yRange[1]) - log(yRange[0]))); - // - // Use both sides as the exponent in 10^exp and we're done. - // y = 10 ^ (log(yRange[1]) - (pct * (log(yRange[1]) - log(yRange[0])))); - var logr0 = utils.log10(yRange[0]); - var logr1 = utils.log10(yRange[1]); - var exponent = logr1 - (pct * (logr1 - logr0)); - var value = Math.pow(utils.LOG_SCALE, exponent); - return value; + // Note reversed yRange, y1 is on top with pct==0. + return utils.logRangeFraction(yRange[1], yRange[0], pct); } }; @@ -2602,25 +2554,21 @@ Dygraph.prototype.computeYAxisRanges_ = function(extremes) { } } - var maxAxisY, minAxisY; - if (logscale) { - if (ypadCompat) { + var maxAxisY = maxY, minAxisY = minY; + if (ypadCompat) { + if (logscale) { maxAxisY = maxY + ypad * span; minAxisY = minY; } else { - var logpad = Math.exp(Math.log(span) * ypad); - maxAxisY = maxY * logpad; - minAxisY = minY / logpad; - } - } else { - maxAxisY = maxY + ypad * span; - minAxisY = minY - ypad * span; - - // Backwards-compatible behavior: Move the span to start or end at zero if it's - // close to zero, but not if avoidMinZero is set. - if (ypadCompat && !this.getBooleanOption("avoidMinZero")) { - if (minAxisY < 0 && minY >= 0) minAxisY = 0; - if (maxAxisY > 0 && maxY <= 0) maxAxisY = 0; + maxAxisY = maxY + ypad * span; + minAxisY = minY - ypad * span; + + // Backwards-compatible behavior: Move the span to start or end at zero if it's + // close to zero, but not if avoidMinZero is set. + if (!this.getBooleanOption("avoidMinZero")) { + if (minAxisY < 0 && minY >= 0) minAxisY = 0; + if (maxAxisY > 0 && maxY <= 0) maxAxisY = 0; + } } } axis.extremeRange = [minAxisY, maxAxisY]; @@ -2634,21 +2582,28 @@ Dygraph.prototype.computeYAxisRanges_ = function(extremes) { // This is a user-set value range for this axis. var y0 = isNullUndefinedOrNaN(axis.valueRange[0]) ? axis.extremeRange[0] : axis.valueRange[0]; var y1 = isNullUndefinedOrNaN(axis.valueRange[1]) ? axis.extremeRange[1] : axis.valueRange[1]; - if (!ypadCompat) { - if (axis.logscale) { - var logpad = Math.exp(Math.log(span) * ypad); - y0 *= logpad; - y1 /= logpad; - } else { - span = y1 - y0; - y0 -= span * ypad; - y1 += span * ypad; - } - } axis.computedValueRange = [y0, y1]; } else { axis.computedValueRange = axis.extremeRange; } + if (!axis.valueWindow && !ypadCompat) { + // When using yRangePad, adjust the upper/lower bounds to add + // padding unless the user has zoomed/panned the Y axis range. + if (logscale) { + y0 = axis.computedValueRange[0]; + y1 = axis.computedValueRange[1]; + var y0pct = ypad / (2 * ypad - 1); + var y1pct = (ypad - 1) / (2 * ypad - 1); + axis.computedValueRange[0] = utils.logRangeFraction(y0, y1, y0pct); + axis.computedValueRange[1] = utils.logRangeFraction(y0, y1, y1pct); + } else { + y0 = axis.computedValueRange[0]; + y1 = axis.computedValueRange[1]; + span = y1 - y0; + axis.computedValueRange[0] = y0 - span * ypad; + axis.computedValueRange[1] = y1 + span * ypad; + } + } if (independentTicks) {