Skip to content

timeseries: keep all digits for step values in scalar cards #5325

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
yatbear merged 3 commits into from
Sep 22, 2021
Merged

timeseries: keep all digits for step values in scalar cards #5325

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f5fadfe
keep 4 significant digits for step values with SI-suffix
yatbear Sep 21, 2021
431cdd1
remove SI format and keep three decimal places
yatbear Sep 22, 2021
aae8e66
only use intl formatter for steps in the cards, not the axes
yatbear Sep 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import {
Formatter,
numberFormatter,
relativeTimeFormatter,
intlNumberFormatter,
siNumberFormatter,
} from '../../../widgets/line_chart_v2/lib/formatter';
import {LineChartComponent} from '../../../widgets/line_chart_v2/line_chart_component';
Expand Down Expand Up @@ -110,7 +111,7 @@ export class ScalarCardComponent<Downloader> {

readonly relativeXFormatter = relativeTimeFormatter;
readonly valueFormatter = numberFormatter;
readonly stepFormatter = siNumberFormatter;
readonly stepFormatter = intlNumberFormatter;

getCustomXFormatter(): Formatter | undefined {
switch (this.xAxisType) {
Expand Down
12 changes: 6 additions & 6 deletions tensorboard/webapp/metrics/views/card_renderer/scalar_card_test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1384,7 +1384,7 @@ describe('scalar card', () => {

assertTooltipRows(fixture, [
['', 'Row 2', '-500', '1,000', anyString, anyString],
['', 'Row 3', '3', '10k', anyString, anyString],
['', 'Row 3', '3', '10,000', anyString, anyString],
['', 'Row 1', '1000', '10', anyString, anyString],
]);
}));
Expand Down Expand Up @@ -1453,7 +1453,7 @@ describe('scalar card', () => {

assertTooltipRows(fixture, [
['', 'Row 1', '1000', '10', anyString, anyString],
['', 'Row 3', '3', '10k', anyString, anyString],
['', 'Row 3', '3', '10,000', anyString, anyString],
['', 'Row 2', '-500', '1,000', anyString, anyString],
]);
}));
Expand Down Expand Up @@ -1523,21 +1523,21 @@ describe('scalar card', () => {
assertTooltipRows(fixture, [
['', 'Row 2', '-500', '1,000', anyString, anyString],
['', 'Row 1', '1000', '0', anyString, anyString],
['', 'Row 3', '3', '10k', anyString, anyString],
['', 'Row 3', '3', '10,000', anyString, anyString],
]);

setCursorLocation(fixture, {x: 500, y: 600});
fixture.detectChanges();
assertTooltipRows(fixture, [
['', 'Row 1', '1000', '0', anyString, anyString],
['', 'Row 2', '-500', '1,000', anyString, anyString],
['', 'Row 3', '3', '10k', anyString, anyString],
['', 'Row 3', '3', '10,000', anyString, anyString],
]);

setCursorLocation(fixture, {x: 10000, y: -100});
fixture.detectChanges();
assertTooltipRows(fixture, [
['', 'Row 3', '3', '10k', anyString, anyString],
['', 'Row 3', '3', '10,000', anyString, anyString],
['', 'Row 2', '-500', '1,000', anyString, anyString],
['', 'Row 1', '1000', '0', anyString, anyString],
]);
Expand All @@ -1548,7 +1548,7 @@ describe('scalar card', () => {
assertTooltipRows(fixture, [
['', 'Row 1', '1000', '0', anyString, anyString],
['', 'Row 2', '-500', '1,000', anyString, anyString],
['', 'Row 3', '3', '10k', anyString, anyString],
['', 'Row 3', '3', '10,000', anyString, anyString],
]);
}));
});
Expand Down
21 changes: 21 additions & 0 deletions tensorboard/webapp/widgets/line_chart_v2/lib/formatter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,27 @@ export const numberFormatter: Formatter = {
formatLong: d3LongFormatter,
};

/**
* ===================
* INTL NUMBER FORMATTER
* ===================
*/

const IntlNumberFormatter = new Intl.NumberFormat(undefined, {
maximumFractionDigits: 3,
});

function formatIntlNumber(x: number): string {
return IntlNumberFormatter.format(x);
}

export const intlNumberFormatter: Formatter = {
formatTick: formatIntlNumber,
formatShort: formatIntlNumber,
formatReadable: formatIntlNumber,
formatLong: formatIntlNumber,
};

/**
* ===================
* SI NUMBER FORMATTER
Expand Down
32 changes: 31 additions & 1 deletion tensorboard/webapp/widgets/line_chart_v2/lib/formatter_test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ limitations under the License.
import {
numberFormatter,
relativeTimeFormatter,
intlNumberFormatter,
siNumberFormatter,
TEST_ONLY,
wallTimeFormatter,
Expand Down Expand Up @@ -167,6 +168,32 @@ describe('line_chart_v2/lib/formatter test', () => {
});
});

describe('#intlNumberFormatter', () => {
for (const {name, fn} of [
{name: 'formatTick', fn: intlNumberFormatter.formatTick},
{name: 'formatShort', fn: intlNumberFormatter.formatShort},
{name: 'formatReadable', fn: intlNumberFormatter.formatReadable},
{name: 'formatLong', fn: intlNumberFormatter.formatLong},
]) {
describe(`#${name}`, () => {
it('formats numbers and keeps three decimal places', () => {
expect(fn(1)).toBe('1');
expect(fn(5)).toBe('5');
expect(fn(-100.4)).toBe('-100.4');
expect(fn(3.01)).toBe('3.01');
expect(fn(9999)).toBe('9,999');
expect(fn(9999.9123)).toBe('9,999.912');
expect(fn(0.09)).toBe('0.09');
expect(fn(0.0005)).toBe('0.001');
expect(fn(0.00005)).toBe('0');
expect(fn(10001)).toBe('10,001');
expect(fn(-10000)).toBe('-10,000');
expect(fn(-1.004e6)).toBe('-1,004,000');
});
});
}
});

describe('#siNumberFormatter', () => {
for (const {name, fn} of [
{name: 'formatTick', fn: siNumberFormatter.formatTick},
Expand All @@ -175,14 +202,17 @@ describe('line_chart_v2/lib/formatter test', () => {
{name: 'formatLong', fn: siNumberFormatter.formatLong},
]) {
describe(`#${name}`, () => {
it('formats without si-suffix for number less than 10k', () => {
it('formats without si-suffix', () => {
expect(fn(1)).toBe('1');
expect(fn(5)).toBe('5');
expect(fn(-100.4)).toBe('-100.4');
expect(fn(3.01)).toBe('3.01');
expect(fn(9999)).toBe('9,999');
expect(fn(9999.9123)).toBe('9,999.912');
expect(fn(0.09)).toBe('0.09');
});

it('formats with si-suffix', () => {
expect(fn(10000)).toBe('10k');
expect(fn(10001)).toBe('10k');
expect(fn(-10000)).toBe('-10k');
Expand Down