Skip to content

Commit

Permalink
Bug fix: Use Standard ticks when dealing with scientific notation (te…
Browse files Browse the repository at this point in the history
…nsorflow#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">
  • Loading branch information
JamesHollyer authored Mar 17, 2023
1 parent eaa8353 commit afd50e2
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,17 @@ function getTicksForLinearScale(

const minorTickVals = scale.ticks([low, high], maxMinorTickCount);
const majorTickVals = scale.ticks([low, high], 2);

// If the numbers are small enough that javascript starts using scientific
// notation the logic here does not work. Also, those numbers normally show up
// well using the standard ticks.
if (
containsScientificNotation(minorTickVals) ||
containsScientificNotation(majorTickVals)
) {
return getStandardTicks(scale, formatter, maxMinorTickCount, lowAndHigh);
}

const minor: MinorTick[] = [];

let numFractionalToKeep = getNumLeadingZerosInFractional(diff);
Expand Down Expand Up @@ -224,9 +235,20 @@ function filterTicksByVisibility(
});
}

function containsScientificNotation(values: number[]): boolean {
for (const value of values) {
if (String(value).includes('e')) {
return true;
}
}
return false;
}

export const AxisUtils = {
getStandardTicks,
getTicksForTemporalScale,
getTicksForLinearScale,
filterTicksByVisibility,
};

export const TEST_ONLY = {containsScientificNotation};
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ limitations under the License.
==============================================================================*/

import {createScale, LinearScale, ScaleType, TemporalScale} from '../lib/scale';
import {AxisUtils} from './line_chart_axis_utils';
import {AxisUtils, TEST_ONLY} from './line_chart_axis_utils';

describe('line_chart_v2/sub_view/axis_utils test', () => {
describe('#getStandardTicks', () => {
Expand Down Expand Up @@ -363,6 +363,32 @@ describe('line_chart_v2/sub_view/axis_utils test', () => {
{value: -0.00001, tickFormattedString: '…0'},
]);
});
it('returns no major ticks for numbers which are transated to scientific notation', () => {
const {major, minor} = AxisUtils.getTicksForLinearScale(
scale,
scale.defaultFormatter,
5,
[0.0000000004, 0.000000009]
);

expect(major).toEqual([]);
expect(minor).toEqual([
{value: 2e-9, tickFormattedString: '2e-9'},
{value: 4e-9, tickFormattedString: '4e-9'},
{value: 6e-9, tickFormattedString: '6e-9'},
{value: 8e-9, tickFormattedString: '8e-9'},
]);
});
});
});

describe('#containsScientificNotation', () => {
it('returns true if the array contains scientific notation', () => {
expect(TEST_ONLY.containsScientificNotation([1, 2e-9, 2])).toBe(true);
});

it('returns false if the array does not contain scientific notation', () => {
expect(TEST_ONLY.containsScientificNotation([1, 4, 2])).toBe(false);
});
});

Expand Down

0 comments on commit afd50e2

Please sign in to comment.