Skip to content

Commit afd50e2

Browse files
authored
Bug fix: Use Standard ticks when dealing with scientific notation (#6247)
* Motivation for features / changes When the differences between the max and min points on an axis are very small we use some special logic to make the axis tick marks more readable. However, this logic assumes the numbers to be decimal and gives NaN values when the numbers take on scientific notation. Fortunately more times scientific notation is used the number become small enough to use regular tick marks. This change simply reverts back to standard tick marks whenever javascript starts using scientific notation. * Screenshots of UI changes Before: <img width="90" alt="Screenshot 2023-03-16 at 9 37 26 PM" src="https://user-images.githubusercontent.com/8672809/225814036-3b622a9c-bc63-47c4-ad09-6eea1b5cf727.png"> After: <img width="72" alt="Screenshot 2023-03-16 at 9 39 40 PM" src="https://user-images.githubusercontent.com/8672809/225814092-48faa8e8-11c8-4ba5-a27f-648dacae75d7.png">
1 parent eaa8353 commit afd50e2

File tree

2 files changed

+49
-1
lines changed

2 files changed

+49
-1
lines changed

tensorboard/webapp/widgets/line_chart_v2/sub_view/line_chart_axis_utils.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,17 @@ function getTicksForLinearScale(
102102

103103
const minorTickVals = scale.ticks([low, high], maxMinorTickCount);
104104
const majorTickVals = scale.ticks([low, high], 2);
105+
106+
// If the numbers are small enough that javascript starts using scientific
107+
// notation the logic here does not work. Also, those numbers normally show up
108+
// well using the standard ticks.
109+
if (
110+
containsScientificNotation(minorTickVals) ||
111+
containsScientificNotation(majorTickVals)
112+
) {
113+
return getStandardTicks(scale, formatter, maxMinorTickCount, lowAndHigh);
114+
}
115+
105116
const minor: MinorTick[] = [];
106117

107118
let numFractionalToKeep = getNumLeadingZerosInFractional(diff);
@@ -224,9 +235,20 @@ function filterTicksByVisibility(
224235
});
225236
}
226237

238+
function containsScientificNotation(values: number[]): boolean {
239+
for (const value of values) {
240+
if (String(value).includes('e')) {
241+
return true;
242+
}
243+
}
244+
return false;
245+
}
246+
227247
export const AxisUtils = {
228248
getStandardTicks,
229249
getTicksForTemporalScale,
230250
getTicksForLinearScale,
231251
filterTicksByVisibility,
232252
};
253+
254+
export const TEST_ONLY = {containsScientificNotation};

tensorboard/webapp/widgets/line_chart_v2/sub_view/line_chart_axis_utils_test.ts

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ limitations under the License.
1414
==============================================================================*/
1515

1616
import {createScale, LinearScale, ScaleType, TemporalScale} from '../lib/scale';
17-
import {AxisUtils} from './line_chart_axis_utils';
17+
import {AxisUtils, TEST_ONLY} from './line_chart_axis_utils';
1818

1919
describe('line_chart_v2/sub_view/axis_utils test', () => {
2020
describe('#getStandardTicks', () => {
@@ -363,6 +363,32 @@ describe('line_chart_v2/sub_view/axis_utils test', () => {
363363
{value: -0.00001, tickFormattedString: '…0'},
364364
]);
365365
});
366+
it('returns no major ticks for numbers which are transated to scientific notation', () => {
367+
const {major, minor} = AxisUtils.getTicksForLinearScale(
368+
scale,
369+
scale.defaultFormatter,
370+
5,
371+
[0.0000000004, 0.000000009]
372+
);
373+
374+
expect(major).toEqual([]);
375+
expect(minor).toEqual([
376+
{value: 2e-9, tickFormattedString: '2e-9'},
377+
{value: 4e-9, tickFormattedString: '4e-9'},
378+
{value: 6e-9, tickFormattedString: '6e-9'},
379+
{value: 8e-9, tickFormattedString: '8e-9'},
380+
]);
381+
});
382+
});
383+
});
384+
385+
describe('#containsScientificNotation', () => {
386+
it('returns true if the array contains scientific notation', () => {
387+
expect(TEST_ONLY.containsScientificNotation([1, 2e-9, 2])).toBe(true);
388+
});
389+
390+
it('returns false if the array does not contain scientific notation', () => {
391+
expect(TEST_ONLY.containsScientificNotation([1, 4, 2])).toBe(false);
366392
});
367393
});
368394

0 commit comments

Comments
 (0)