From 2c3c5b09f8ae395d1cc2641c0f26835d97b6261c Mon Sep 17 00:00:00 2001 From: Stephan Lee Date: Fri, 21 May 2021 13:00:41 -0700 Subject: [PATCH 1/2] line chart: improve major axis for numbers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the extents are small and is close to 0, we were falling into the case where major axes were missing and minor axis formats were simply off. They were supposed to have truncated number but instead had `.…0000535` because we were removing common prefix between "0" and "0.0000535" and blindly adding `…` in it. This change addresses the corner case by adding test specs and special casing major ticks being `0`s. --- .../sub_view/line_chart_axis_utils.ts | 23 ++++++++-- .../sub_view/line_chart_axis_utils_test.ts | 46 +++++++++++++++++++ 2 files changed, 66 insertions(+), 3 deletions(-) diff --git a/tensorboard/webapp/widgets/line_chart_v2/sub_view/line_chart_axis_utils.ts b/tensorboard/webapp/widgets/line_chart_v2/sub_view/line_chart_axis_utils.ts index 53a05f139b..78cef5d429 100644 --- a/tensorboard/webapp/widgets/line_chart_v2/sub_view/line_chart_axis_utils.ts +++ b/tensorboard/webapp/widgets/line_chart_v2/sub_view/line_chart_axis_utils.ts @@ -101,10 +101,27 @@ export function getTicksForLinearScale( } const minorTickVals = scale.ticks([low, high], maxMinorTickCount); - const numFractionalToKeep = getNumLeadingZerosInFractional(diff); const majorTickVals = scale.ticks([low, high], 2); const minor: MinorTick[] = []; + let numFractionalToKeep = getNumLeadingZerosInFractional(diff); + + // In case the low and highs are 0 and [0, 1), e.g., [0, 0.0001], we would + // like to keep a bit more fractionals than other cases. For example, For + // above example, the `diff` is `0.0001` and `numFractionalToKeep` is + // 3 (number of leading zeros after decimals). That would effectively make + // majorTickVal just `0` and provide very awkward UX. For that case, we want + // to keep one extra fractional number. + if ( + diff < 1 && + majorTickVals.every((tickVal) => { + const absTickVal = Math.abs(tickVal); + return absTickVal >= 0 && absTickVal < 1; + }) + ) { + numFractionalToKeep += 1; + } + const majorTickValMap = new Map(); for (const val of majorTickVals) { const [whole, fractional = ''] = String(val).split('.', 2); @@ -123,8 +140,8 @@ export function getTicksForLinearScale( const maximumDiff = 10 * Math.pow(10, -numFractionalToKeep); for (const val of minorTickVals) { - for (const flooredMajorVal of majorTickValMap.keys()) { - const diff = Math.abs(val - flooredMajorVal); + for (const flooredMajorVal of [...majorTickValMap.keys()].reverse()) { + const diff = val - flooredMajorVal; if (diff >= 0 && diff < maximumDiff) { // `diff` can have very minute number because of IEEE 754. const remainder = String(val).slice(String(flooredMajorVal).length); diff --git a/tensorboard/webapp/widgets/line_chart_v2/sub_view/line_chart_axis_utils_test.ts b/tensorboard/webapp/widgets/line_chart_v2/sub_view/line_chart_axis_utils_test.ts index 491fcd532b..d4778605be 100644 --- a/tensorboard/webapp/widgets/line_chart_v2/sub_view/line_chart_axis_utils_test.ts +++ b/tensorboard/webapp/widgets/line_chart_v2/sub_view/line_chart_axis_utils_test.ts @@ -288,6 +288,52 @@ describe('line_chart_v2/sub_view/axis_utils test', () => { {value: 1.94516, tickFormattedString: '…6'}, ]); }); + + it('handles 0 and small number close to 0 well', () => { + const {major, minor} = getTicksForLinearScale( + scale, + scale.defaultFormatter, + 2, + [0, 0.0001999] + ); + expect(major).toEqual([ + {start: 0, tickFormattedString: '0'}, + {start: 0.0001, tickFormattedString: '1e-4'}, + ]); + expect(minor).toEqual([ + {value: 0, tickFormattedString: '…0'}, + {value: 0.0001, tickFormattedString: '…0'}, + ]); + }); + + it('handles extent close to 0s well', () => { + const {major, minor} = getTicksForLinearScale( + scale, + scale.defaultFormatter, + 2, + [0.000001, 0.0001999] + ); + expect(major).toEqual([{start: 0.0001, tickFormattedString: '1e-4'}]); + expect(minor).toEqual([{value: 0.0001, tickFormattedString: '…0'}]); + }); + + it('handles negative extent close to 0s well', () => { + const {major, minor} = getTicksForLinearScale( + scale, + scale.defaultFormatter, + 2, + [-0.000001999, -0.00001] + ); + + expect(major).toEqual([ + {start: -0.000005, tickFormattedString: '-5e-6'}, + {start: -0.00001, tickFormattedString: '-1e-5'}, + ]); + expect(minor).toEqual([ + {value: -0.000005, tickFormattedString: '…5'}, + {value: -0.00001, tickFormattedString: '…0'}, + ]); + }); }); }); }); From 97e78c1778f90eb6cbcc18d87eb0ca1f78472199 Mon Sep 17 00:00:00 2001 From: Stephan Lee Date: Mon, 24 May 2021 09:47:07 -0700 Subject: [PATCH 2/2] hmm --- .../sub_view/line_chart_axis_utils.ts | 24 +++++++++--- .../sub_view/line_chart_axis_utils_test.ts | 37 ++++++++++++++++++- 2 files changed, 53 insertions(+), 8 deletions(-) diff --git a/tensorboard/webapp/widgets/line_chart_v2/sub_view/line_chart_axis_utils.ts b/tensorboard/webapp/widgets/line_chart_v2/sub_view/line_chart_axis_utils.ts index 78cef5d429..211b620f25 100644 --- a/tensorboard/webapp/widgets/line_chart_v2/sub_view/line_chart_axis_utils.ts +++ b/tensorboard/webapp/widgets/line_chart_v2/sub_view/line_chart_axis_utils.ts @@ -133,7 +133,8 @@ export function getTicksForLinearScale( // Put it in the middle. If the flooredNumber is 231.041, then put the axis label // at 231.0415 which is not the most ideal but certainly better than 231.041. start: flooredNumber, - tickFormattedString: formatter.formatShort(flooredNumber), + tickFormattedString: + flooredNumber === 0 ? '—' : formatter.formatShort(flooredNumber), }); } @@ -144,11 +145,22 @@ export function getTicksForLinearScale( const diff = val - flooredMajorVal; if (diff >= 0 && diff < maximumDiff) { // `diff` can have very minute number because of IEEE 754. - const remainder = String(val).slice(String(flooredMajorVal).length); - minor.push({ - value: val, - tickFormattedString: `…${remainder || '0'}`, - }); + + // When major axis is `0`, there is no right way to truncate it. Use the + // real formatter in that case. + if (flooredMajorVal === 0) { + minor.push({ + value: val, + tickFormattedString: formatter.formatTick(val), + }); + } else { + const remainder = String(val).slice(String(flooredMajorVal).length); + minor.push({ + value: val, + tickFormattedString: `…${remainder || '0'}`, + }); + } + break; } } diff --git a/tensorboard/webapp/widgets/line_chart_v2/sub_view/line_chart_axis_utils_test.ts b/tensorboard/webapp/widgets/line_chart_v2/sub_view/line_chart_axis_utils_test.ts index d4778605be..3075feb23d 100644 --- a/tensorboard/webapp/widgets/line_chart_v2/sub_view/line_chart_axis_utils_test.ts +++ b/tensorboard/webapp/widgets/line_chart_v2/sub_view/line_chart_axis_utils_test.ts @@ -208,6 +208,20 @@ describe('line_chart_v2/sub_view/axis_utils test', () => { ]); }); + it('handles extents with 0 and larger number', () => { + const {major, minor} = getTicksForLinearScale( + scale, + scale.defaultFormatter, + 2, + [0, 3.2105] + ); + expect(major).toEqual([]); + expect(minor).toEqual([ + {value: 0, tickFormattedString: '0'}, + {value: 2, tickFormattedString: '2'}, + ]); + }); + describe('very small differences', () => { it('creates a major tick since very long minor tick labels are not legible', () => { const {major, minor} = getTicksForLinearScale( @@ -297,12 +311,31 @@ describe('line_chart_v2/sub_view/axis_utils test', () => { [0, 0.0001999] ); expect(major).toEqual([ - {start: 0, tickFormattedString: '0'}, + {start: 0, tickFormattedString: '—'}, + {start: 0.0001, tickFormattedString: '1e-4'}, + ]); + expect(minor).toEqual([ + {value: 0, tickFormattedString: '0'}, + {value: 0.0001, tickFormattedString: '…0'}, + ]); + }); + + it('handles 0 and small number close to 0 well (more minor ticks)', () => { + const {major, minor} = getTicksForLinearScale( + scale, + scale.defaultFormatter, + 4, + [0, 0.00019999999495] + ); + expect(major).toEqual([ + {start: 0, tickFormattedString: '—'}, {start: 0.0001, tickFormattedString: '1e-4'}, ]); expect(minor).toEqual([ - {value: 0, tickFormattedString: '…0'}, + {value: 0, tickFormattedString: '0'}, + {value: 0.00005, tickFormattedString: '5e-5'}, {value: 0.0001, tickFormattedString: '…0'}, + {value: 0.00015, tickFormattedString: '…5'}, ]); });