Skip to content

Commit 0d29898

Browse files
JamesHollyerJamesHollyer
committed
Refactor: Handle Smoothed column logic in DataTable
1 parent 0a2632d commit 0d29898

File tree

8 files changed

+72
-75
lines changed

8 files changed

+72
-75
lines changed

‎tensorboard/webapp/metrics/store/metrics_reducers.ts

+1-16
Original file line numberDiff line numberDiff line change
@@ -1100,24 +1100,9 @@ const reducer = createReducer(
11001100
};
11011101
}
11021102

1103-
// TODO(@jameshollyer): remove this logic with smoothing refactor *******
1104-
let orderAdjustedForSmoothed = newOrder;
1105-
const newSmoothedColumnIndex = newOrder.indexOf(ColumnHeaders.SMOOTHED);
1106-
const oldSmoothedColumnIndex = state.singleSelectionHeaders.indexOf(
1107-
ColumnHeaders.SMOOTHED
1108-
);
1109-
1110-
if (newSmoothedColumnIndex < 0 && oldSmoothedColumnIndex > 0) {
1111-
orderAdjustedForSmoothed = newOrder
1112-
.slice(0, oldSmoothedColumnIndex)
1113-
.concat([ColumnHeaders.SMOOTHED])
1114-
.concat(newOrder.slice(oldSmoothedColumnIndex, newOrder.length));
1115-
}
1116-
// *********************************************************************
1117-
11181103
return {
11191104
...state,
1120-
singleSelectionHeaders: orderAdjustedForSmoothed,
1105+
singleSelectionHeaders: newOrder,
11211106
};
11221107
}),
11231108
on(actions.metricsToggleVisiblePlugin, (state, {plugin}) => {

‎tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ng.html

+1
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,7 @@
178178
[dataHeaders]="dataHeaders"
179179
[sortingInfo]="sortingInfo"
180180
[columnCustomizationEnabled]="columnCustomizationEnabled"
181+
[smoothingEnabled]="smoothingEnabled"
181182
(sortDataBy)="sortDataBy($event)"
182183
(orderColumns)="reorderColumnHeaders.emit($event)"
183184
>

‎tensorboard/webapp/metrics/views/card_renderer/scalar_card_container.ts

+6-23
Original file line numberDiff line numberDiff line change
@@ -492,34 +492,17 @@ export class ScalarCardContainer implements CardRenderer, OnInit, OnDestroy {
492492
);
493493

494494
this.columnHeaders$ = combineLatest([
495-
this.smoothingEnabled$,
496495
this.stepOrLinkedTimeSelection$,
497496
this.store.select(getSingleSelectionHeaders),
498497
this.store.select(getRangeSelectionHeaders),
499498
]).pipe(
500-
map(
501-
([
502-
smoothingEnabled,
503-
timeSelection,
504-
singleSelectionHeaders,
505-
rangeSelectionHeaders,
506-
]) => {
507-
if (timeSelection === null || timeSelection.end === null) {
508-
if (!smoothingEnabled) {
509-
// Return single selection headers without smoothed header.
510-
const indexOfSmoothed = singleSelectionHeaders.indexOf(
511-
ColumnHeaders.SMOOTHED
512-
);
513-
return singleSelectionHeaders
514-
.slice(0, indexOfSmoothed)
515-
.concat(singleSelectionHeaders.slice(indexOfSmoothed + 1));
516-
}
517-
return singleSelectionHeaders;
518-
} else {
519-
return rangeSelectionHeaders;
520-
}
499+
map(([timeSelection, singleSelectionHeaders, rangeSelectionHeaders]) => {
500+
if (timeSelection === null || timeSelection.end === null) {
501+
return singleSelectionHeaders;
502+
} else {
503+
return rangeSelectionHeaders;
521504
}
522-
)
505+
})
523506
);
524507

525508
this.chartMetadataMap$ = partitionedSeries$.pipe(

‎tensorboard/webapp/metrics/views/card_renderer/scalar_card_data_table.ts

+2
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ import {
3939
[data]="getTimeSelectionTableData()"
4040
[sortingInfo]="sortingInfo"
4141
[columnCustomizationEnabled]="columnCustomizationEnabled"
42+
[smoothingEnabled]="smoothingEnabled"
4243
(sortDataBy)="sortDataBy.emit($event)"
4344
(orderColumns)="orderColumns.emit($event)"
4445
></tb-data-table>
@@ -52,6 +53,7 @@ export class ScalarCardDataTable {
5253
@Input() dataHeaders!: ColumnHeaders[];
5354
@Input() sortingInfo!: SortingInfo;
5455
@Input() columnCustomizationEnabled!: boolean;
56+
@Input() smoothingEnabled!: boolean;
5557

5658
@Output() sortDataBy = new EventEmitter<SortingInfo>();
5759
@Output() orderColumns = new EventEmitter<ColumnHeaders[]>();

‎tensorboard/webapp/metrics/views/card_renderer/scalar_card_test.ts

+3-34
Original file line numberDiff line numberDiff line change
@@ -2628,6 +2628,7 @@ describe('scalar card', () => {
26282628
RUN: 'run1',
26292629
STEP: 2,
26302630
VALUE: 10,
2631+
SMOOTHED: 10,
26312632
},
26322633
{
26332634
id: 'run2',
@@ -2636,6 +2637,7 @@ describe('scalar card', () => {
26362637
RUN: 'run2',
26372638
STEP: 2,
26382639
VALUE: 10,
2640+
SMOOTHED: 10,
26392641
},
26402642
]);
26412643
}));
@@ -3011,7 +3013,7 @@ describe('scalar card', () => {
30113013
expect(data[1].RUN).toEqual('200 test alias 2/Run2 name');
30123014
}));
30133015

3014-
it('adds smoothed column header when smoothed is enabled', fakeAsync(() => {
3016+
it('edits smoothed value when smoothed is enabled', fakeAsync(() => {
30153017
store.overrideSelector(selectors.getMetricsScalarSmoothing, 0.8);
30163018

30173019
const runToSeries = {
@@ -3053,39 +3055,6 @@ describe('scalar card', () => {
30533055
).toBe(6.000000000000001);
30543056
}));
30553057

3056-
it('does not add smoothed column header when smoothed is disabled', fakeAsync(() => {
3057-
store.overrideSelector(selectors.getMetricsScalarSmoothing, 0);
3058-
3059-
const runToSeries = {
3060-
run1: [{wallTime: 1, value: 1, step: 10}],
3061-
};
3062-
provideMockCardRunToSeriesData(
3063-
selectSpy,
3064-
PluginType.SCALARS,
3065-
'card1',
3066-
null /* metadataOverride */,
3067-
runToSeries
3068-
);
3069-
store.overrideSelector(
3070-
selectors.getCurrentRouteRunSelection,
3071-
new Map([['run1', true]])
3072-
);
3073-
store.overrideSelector(getMetricsLinkedTimeSelection, {
3074-
start: {step: 20},
3075-
end: null,
3076-
});
3077-
3078-
const fixture = createComponent('card1');
3079-
fixture.detectChanges();
3080-
const scalarCardDataTable = fixture.debugElement.query(
3081-
By.directive(ScalarCardDataTable)
3082-
);
3083-
3084-
expect(scalarCardDataTable.componentInstance.dataHeaders).not.toContain(
3085-
ColumnHeaders.SMOOTHED
3086-
);
3087-
}));
3088-
30893058
it('orders data ascending', fakeAsync(() => {
30903059
const runToSeries = {
30913060
run1: [{wallTime: 1, value: 2, step: 1}],

‎tensorboard/webapp/widgets/data_table/data_table_component.ng.html

+8-2
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,10 @@
2020
<!-- This header is intentionally left blank for the color column -->
2121
</th>
2222
<ng-container *ngFor="let header of headers;">
23-
<th (click)="headerClicked(header)">
23+
<th
24+
*ngIf="smoothingEnabled || header !== ColumnHeaders.SMOOTHED"
25+
(click)="headerClicked(header)"
26+
>
2427
<div
2528
[draggable]="columnCustomizationEnabled"
2629
(dragstart)="dragStart(header)"
@@ -67,7 +70,10 @@
6770
<span [style.backgroundColor]="runData.COLOR"></span>
6871
</td>
6972
<ng-container *ngFor="let header of headers;">
70-
<td [ngSwitch]="header">
73+
<td
74+
*ngIf="smoothingEnabled || header !== ColumnHeaders.SMOOTHED"
75+
[ngSwitch]="header"
76+
>
7177
<div *ngSwitchCase="ColumnHeaders.VALUE_CHANGE" class="cell">
7278
<ng-container
7379
*ngTemplateOutlet="arrow; context: {$implicit:runData.VALUE_CHANGE}"

‎tensorboard/webapp/widgets/data_table/data_table_component.ts

+1
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ export class DataTableComponent implements OnDestroy {
5555
@Input() data!: SelectedStepRunData[];
5656
@Input() sortingInfo!: SortingInfo;
5757
@Input() columnCustomizationEnabled!: boolean;
58+
@Input() smoothingEnabled!: boolean;
5859

5960
@Output() sortDataBy = new EventEmitter<SortingInfo>();
6061
@Output() orderColumns = new EventEmitter<ColumnHeaders[]>();

‎tensorboard/webapp/widgets/data_table/data_table_test.ts

+50
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import {DataTableComponent} from './data_table_component';
3333
[headers]="headers"
3434
[data]="data"
3535
[sortingInfo]="sortingInfo"
36+
[smoothingEnabled]="smoothingEnabled"
3637
(sortDataBy)="sortDataBy($event)"
3738
(orderColumns)="orderColumns($event)"
3839
></tb-data-table>
@@ -45,6 +46,7 @@ class TestableComponent {
4546
@Input() headers!: ColumnHeaders[];
4647
@Input() data!: SelectedStepRunData[];
4748
@Input() sortingInfo!: SortingInfo;
49+
@Input() smoothingEnabled!: boolean;
4850

4951
@Input() sortDataBy!: (sortingInfo: SortingInfo) => void;
5052
@Input() orderColumns!: (newOrder: ColumnHeaders[]) => void;
@@ -64,6 +66,7 @@ describe('data table', () => {
6466
headers?: ColumnHeaders[];
6567
data?: SelectedStepRunData[];
6668
sortingInfo?: SortingInfo;
69+
smoothingEnabled?: boolean;
6770
}): ComponentFixture<TestableComponent> {
6871
const fixture = TestBed.createComponent(TestableComponent);
6972

@@ -74,6 +77,11 @@ describe('data table', () => {
7477
order: SortingOrder.ASCENDING,
7578
};
7679

80+
fixture.componentInstance.smoothingEnabled =
81+
input.smoothingEnabled === undefined ? true : input.smoothingEnabled;
82+
83+
console.log('enabled?', fixture.componentInstance.smoothingEnabled);
84+
7785
sortDataBySpy = jasmine.createSpy();
7886
fixture.componentInstance.sortDataBy = sortDataBySpy;
7987

@@ -105,6 +113,7 @@ describe('data table', () => {
105113
ColumnHeaders.MIN_VALUE,
106114
ColumnHeaders.MAX_VALUE,
107115
ColumnHeaders.PERCENTAGE_CHANGE,
116+
ColumnHeaders.SMOOTHED,
108117
],
109118
});
110119
fixture.detectChanges();
@@ -134,6 +143,7 @@ describe('data table', () => {
134143
.queryAll(By.css('mat-icon'))[0]
135144
.nativeElement.getAttribute('svgIcon')
136145
).toBe('change_history_24px');
146+
expect(headerElements[13].nativeElement.innerText).toBe('Smoothed');
137147
});
138148

139149
it('displays data in order', () => {
@@ -151,6 +161,7 @@ describe('data table', () => {
151161
ColumnHeaders.MIN_VALUE,
152162
ColumnHeaders.MAX_VALUE,
153163
ColumnHeaders.PERCENTAGE_CHANGE,
164+
ColumnHeaders.SMOOTHED,
154165
],
155166
data: [
156167
{
@@ -167,6 +178,7 @@ describe('data table', () => {
167178
MIN_VALUE: 1,
168179
MAX_VALUE: 500,
169180
PERCENTAGE_CHANGE: 0.3,
181+
SMOOTHED: 2,
170182
},
171183
],
172184
});
@@ -199,6 +211,7 @@ describe('data table', () => {
199211
.queryAll(By.css('mat-icon'))[0]
200212
.nativeElement.getAttribute('svgIcon')
201213
).toBe('arrow_upward_24px');
214+
expect(dataElements[13].nativeElement.innerText).toBe('2');
202215
});
203216

204217
it('displays nothing when no data is available', () => {
@@ -400,4 +413,41 @@ describe('data table', () => {
400413
ColumnHeaders.RUN,
401414
]);
402415
});
416+
417+
it('does not display Smoothed column when smoothingEnabled is false', () => {
418+
const fixture = createComponent({
419+
headers: [
420+
ColumnHeaders.VALUE,
421+
ColumnHeaders.RUN,
422+
ColumnHeaders.SMOOTHED,
423+
ColumnHeaders.STEP,
424+
],
425+
data: [
426+
{
427+
id: 'someid',
428+
RUN: 'run name',
429+
VALUE: 3,
430+
STEP: 1,
431+
SMOOTHED: 2,
432+
},
433+
],
434+
smoothingEnabled: false,
435+
});
436+
fixture.detectChanges();
437+
const headerElements = fixture.debugElement.queryAll(By.css('th'));
438+
const dataElements = fixture.debugElement.queryAll(By.css('td'));
439+
440+
// The first header should always be blank as it is the run color column.
441+
expect(headerElements[0].nativeElement.innerText).toBe('');
442+
expect(headerElements[1].nativeElement.innerText).toBe('Value');
443+
expect(headerElements[2].nativeElement.innerText).toBe('Run');
444+
expect(headerElements[3].nativeElement.innerText).toBe('Step');
445+
expect(headerElements.length).toBe(4);
446+
447+
expect(dataElements[0].nativeElement.innerText).toBe('');
448+
expect(dataElements[1].nativeElement.innerText).toBe('3');
449+
expect(dataElements[2].nativeElement.innerText).toBe('run name');
450+
expect(dataElements[3].nativeElement.innerText).toBe('1');
451+
expect(dataElements.length).toBe(4);
452+
});
403453
});

0 commit comments

Comments
 (0)