Commit 831fae0c authored by Phillip Kuznetsov's avatar Phillip Kuznetsov
Browse files

Fix the bad Nan rendering commit

Summary: I messed up the IsNan commit. Turns out that anything that's not a float or an int evals IsNan to true - meaning anytime you pass in a Numeral object it is NaN, although semantically it is not. Fixed now to check if the output of numeral is NaN instead.

Test Plan: tested with the golden queries - no NaNs and more interesting numbers than just 0.

Reviewers: michelle, zasgar, #engineering

Reviewed By: michelle, #engineering

Differential Revision: https://phab.corp.pixielabs.ai/D2311

GitOrigin-RevId: 05fe2fd60891d685777d5318b649c10ea3ff004d
parent 0bcc0925
No related merge requests found
Showing with 36 additions and 9 deletions
+36 -9
......@@ -53,14 +53,6 @@ function formatInt64Data(val: string): string {
return numeral(val).format('0,0');
}
function formatFloat64Data(val: number): string {
// Floating point division of 0 can yield 2e-380 which numeral considers a NaN, this just ignores the error.
let num = numeral(val);
if (isNaN(num)) {
num = numeral(0);
}
return num.format('0[.]00');
}
function extractData(colType: string, col: any, rowIdx): string {
switch (colType) {
......@@ -77,7 +69,7 @@ function extractData(colType: string, col: any, rowIdx): string {
const v = col.uint128Data.data[rowIdx];
return formatUInt128(v.high, v.low);
case 'FLOAT64':
return formatFloat64Data(col.float64Data.data[rowIdx]);
return FormatData.formatFloat64Data(col.float64Data.data[rowIdx]);
case 'BOOLEAN':
return col.booleanData.data[rowIdx] ? 'true' : 'false';
default:
......
......@@ -127,3 +127,23 @@ describe('<JSONData/> test', () => {
expect(wrapper.find(FormatData.JSONData).at(1).props().indentation).toEqual(1);
});
});
describe('formatFloat64Data test', () => {
it('should accept decimal-rendered scientific notation', () => {
// 1e-6 renders to "0.000001" in numeral.js internally
expect(FormatData.formatFloat64Data(1e-6)).toEqual('0');
});
it('should accept scientific notation rendered scientific notation', () => {
// 1e-6 renders to '1e-7' internally in numeral.js, usually throws NaNs
expect(FormatData.formatFloat64Data(1e-7)).toEqual('0');
});
it('should render NaNs correctly', () => {
expect(FormatData.formatFloat64Data(NaN)).toEqual('NaN');
});
it('should render normal floats correctly', () => {
expect(FormatData.formatFloat64Data(123.456)).toEqual('123.46');
});
it('should render huge floats correctly', () => {
expect(FormatData.formatFloat64Data(123.456789101)).toEqual('123.46');
});
});
import * as numeral from 'numeral';
import * as React from 'react';
import './format-data.scss';
......@@ -12,6 +13,20 @@ interface JSONDataProps {
multiline?: boolean;
}
export function formatFloat64Data(val: number): string {
// Numeral.js doesn't actually format NaNs, it ignores them.
if (isNaN(val)) {
return 'NaN';
}
let num = numeral(val).format('0[.]00');
// Numeral.js doesn't have a catch for abs-value decimals less than 1e-6.
if (num === 'NaN' && Math.abs(val) < 1e-6) {
num = formatFloat64Data(0);
}
return num;
}
export function looksLikeLatencyCol(colName: string, colType: string) {
if (colType !== 'FLOAT64') {
return false;
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment