Refactor: Handle Smoothed column logic in DataTable (tensorflow#6094)

* Motivation for features / changes
The smoothed column in the scalar card data table does not render when
smoothing is disabled. The logic for removing this column previously
occurred in the ScalarCardContainer where it removed the column entirely
from the list of headers. This causes some strange behavior which needs
to be handled when dealing with changing the order of the columns.

This change removes the special treatment of the Smoothed column
everywhere except for when it is rendered.

* Screenshots of UI changes
No changes.

Author: JamesHollyer

tensorboard/webapp/metrics/store/metrics_reducers.ts

@@ -1100,24 +1100,9 @@ const reducer = createReducer(
       };
     }
 
-    // TODO(@jameshollyer): remove this logic with smoothing refactor *******
-    let orderAdjustedForSmoothed = newOrder;
-    const newSmoothedColumnIndex = newOrder.indexOf(ColumnHeaders.SMOOTHED);
-    const oldSmoothedColumnIndex = state.singleSelectionHeaders.indexOf(
-      ColumnHeaders.SMOOTHED
-    );
-
-    if (newSmoothedColumnIndex < 0 && oldSmoothedColumnIndex > 0) {
-      orderAdjustedForSmoothed = newOrder
-        .slice(0, oldSmoothedColumnIndex)
-        .concat([ColumnHeaders.SMOOTHED])
-        .concat(newOrder.slice(oldSmoothedColumnIndex, newOrder.length));
-    }
-    // *********************************************************************
-
     return {
       ...state,
-      singleSelectionHeaders: orderAdjustedForSmoothed,
+      singleSelectionHeaders: newOrder,
     };
   }),
   on(actions.metricsToggleVisiblePlugin, (state, {plugin}) => {

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

@@ -178,6 +178,7 @@
       [dataHeaders]="dataHeaders"
       [sortingInfo]="sortingInfo"
       [columnCustomizationEnabled]="columnCustomizationEnabled"
+      [smoothingEnabled]="smoothingEnabled"
       (sortDataBy)="sortDataBy($event)"
       (orderColumns)="reorderColumnHeaders.emit($event)"
     >

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

@@ -492,34 +492,17 @@ export class ScalarCardContainer implements CardRenderer, OnInit, OnDestroy {
     );
 
     this.columnHeaders$ = combineLatest([
-      this.smoothingEnabled$,
       this.stepOrLinkedTimeSelection$,
       this.store.select(getSingleSelectionHeaders),
       this.store.select(getRangeSelectionHeaders),
     ]).pipe(
-      map(
-        ([
-          smoothingEnabled,
-          timeSelection,
-          singleSelectionHeaders,
-          rangeSelectionHeaders,
-        ]) => {
-          if (timeSelection === null || timeSelection.end === null) {
-            if (!smoothingEnabled) {
-              // Return single selection headers without smoothed header.
-              const indexOfSmoothed = singleSelectionHeaders.indexOf(
-                ColumnHeaders.SMOOTHED
-              );
-              return singleSelectionHeaders
-                .slice(0, indexOfSmoothed)
-                .concat(singleSelectionHeaders.slice(indexOfSmoothed + 1));
-            }
-            return singleSelectionHeaders;
-          } else {
-            return rangeSelectionHeaders;
-          }
+      map(([timeSelection, singleSelectionHeaders, rangeSelectionHeaders]) => {
+        if (timeSelection === null || timeSelection.end === null) {
+          return singleSelectionHeaders;
+        } else {
+          return rangeSelectionHeaders;
         }
-      )
+      })
     );
 
     this.chartMetadataMap$ = partitionedSeries$.pipe(

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

@@ -39,6 +39,7 @@ import {
       [data]="getTimeSelectionTableData()"
       [sortingInfo]="sortingInfo"
       [columnCustomizationEnabled]="columnCustomizationEnabled"
+      [smoothingEnabled]="smoothingEnabled"
       (sortDataBy)="sortDataBy.emit($event)"
       (orderColumns)="orderColumns.emit($event)"
     ></tb-data-table>
@@ -52,6 +53,7 @@ export class ScalarCardDataTable {
   @Input() dataHeaders!: ColumnHeaders[];
   @Input() sortingInfo!: SortingInfo;
   @Input() columnCustomizationEnabled!: boolean;
+  @Input() smoothingEnabled!: boolean;
 
   @Output() sortDataBy = new EventEmitter<SortingInfo>();
   @Output() orderColumns = new EventEmitter<ColumnHeaders[]>();

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

@@ -2628,6 +2628,7 @@ describe('scalar card', () => {
           RUN: 'run1',
           STEP: 2,
           VALUE: 10,
+          SMOOTHED: 10,
         },
         {
           id: 'run2',
@@ -2636,6 +2637,7 @@ describe('scalar card', () => {
           RUN: 'run2',
           STEP: 2,
           VALUE: 10,
+          SMOOTHED: 10,
         },
       ]);
     }));
@@ -3011,7 +3013,7 @@ describe('scalar card', () => {
       expect(data[1].RUN).toEqual('200 test alias 2/Run2 name');
     }));
 
-    it('adds smoothed column header when smoothed is enabled', fakeAsync(() => {
+    it('edits smoothed value when smoothed is enabled', fakeAsync(() => {
       store.overrideSelector(selectors.getMetricsScalarSmoothing, 0.8);
 
       const runToSeries = {
@@ -3053,39 +3055,6 @@ describe('scalar card', () => {
       ).toBe(6.000000000000001);
     }));
 
-    it('does not add smoothed column header when smoothed is disabled', fakeAsync(() => {
-      store.overrideSelector(selectors.getMetricsScalarSmoothing, 0);
-
-      const runToSeries = {
-        run1: [{wallTime: 1, value: 1, step: 10}],
-      };
-      provideMockCardRunToSeriesData(
-        selectSpy,
-        PluginType.SCALARS,
-        'card1',
-        null /* metadataOverride */,
-        runToSeries
-      );
-      store.overrideSelector(
-        selectors.getCurrentRouteRunSelection,
-        new Map([['run1', true]])
-      );
-      store.overrideSelector(getMetricsLinkedTimeSelection, {
-        start: {step: 20},
-        end: null,
-      });
-
-      const fixture = createComponent('card1');
-      fixture.detectChanges();
-      const scalarCardDataTable = fixture.debugElement.query(
-        By.directive(ScalarCardDataTable)
-      );
-
-      expect(scalarCardDataTable.componentInstance.dataHeaders).not.toContain(
-        ColumnHeaders.SMOOTHED
-      );
-    }));
-
     it('orders data ascending', fakeAsync(() => {
       const runToSeries = {
         run1: [{wallTime: 1, value: 2, step: 1}],

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

@@ -20,7 +20,10 @@
           <!-- This header is intentionally left blank for the color column -->
         </th>
         <ng-container *ngFor="let header of headers;">
-          <th (click)="headerClicked(header)">
+          <th
+            *ngIf="smoothingEnabled || header !== ColumnHeaders.SMOOTHED"
+            (click)="headerClicked(header)"
+          >
             <div
               [draggable]="columnCustomizationEnabled"
               (dragstart)="dragStart(header)"
@@ -67,7 +70,10 @@
             <span [style.backgroundColor]="runData.COLOR"></span>
           </td>
           <ng-container *ngFor="let header of headers;">
-            <td [ngSwitch]="header">
+            <td
+              *ngIf="smoothingEnabled || header !== ColumnHeaders.SMOOTHED"
+              [ngSwitch]="header"
+            >
               <div *ngSwitchCase="ColumnHeaders.VALUE_CHANGE" class="cell">
                 <ng-container
                   *ngTemplateOutlet="arrow; context: {$implicit:runData.VALUE_CHANGE}"

tensorboard/webapp/widgets/data_table/data_table_component.ts

@@ -55,6 +55,7 @@ export class DataTableComponent implements OnDestroy {
   @Input() data!: SelectedStepRunData[];
   @Input() sortingInfo!: SortingInfo;
   @Input() columnCustomizationEnabled!: boolean;
+  @Input() smoothingEnabled!: boolean;
 
   @Output() sortDataBy = new EventEmitter<SortingInfo>();
   @Output() orderColumns = new EventEmitter<ColumnHeaders[]>();

tensorboard/webapp/widgets/data_table/data_table_test.ts

@@ -33,6 +33,7 @@ import {DataTableComponent} from './data_table_component';
       [headers]="headers"
       [data]="data"
       [sortingInfo]="sortingInfo"
+      [smoothingEnabled]="smoothingEnabled"
       (sortDataBy)="sortDataBy($event)"
       (orderColumns)="orderColumns($event)"
     ></tb-data-table>
@@ -45,6 +46,7 @@ class TestableComponent {
   @Input() headers!: ColumnHeaders[];
   @Input() data!: SelectedStepRunData[];
   @Input() sortingInfo!: SortingInfo;
+  @Input() smoothingEnabled!: boolean;
 
   @Input() sortDataBy!: (sortingInfo: SortingInfo) => void;
   @Input() orderColumns!: (newOrder: ColumnHeaders[]) => void;
@@ -64,6 +66,7 @@ describe('data table', () => {
     headers?: ColumnHeaders[];
     data?: SelectedStepRunData[];
     sortingInfo?: SortingInfo;
+    smoothingEnabled?: boolean;
   }): ComponentFixture<TestableComponent> {
     const fixture = TestBed.createComponent(TestableComponent);
 
@@ -74,6 +77,9 @@ describe('data table', () => {
       order: SortingOrder.ASCENDING,
     };
 
+    fixture.componentInstance.smoothingEnabled =
+      input.smoothingEnabled === undefined ? true : input.smoothingEnabled;
+
     sortDataBySpy = jasmine.createSpy();
     fixture.componentInstance.sortDataBy = sortDataBySpy;
 
@@ -105,6 +111,7 @@ describe('data table', () => {
         ColumnHeaders.MIN_VALUE,
         ColumnHeaders.MAX_VALUE,
         ColumnHeaders.PERCENTAGE_CHANGE,
+        ColumnHeaders.SMOOTHED,
       ],
     });
     fixture.detectChanges();
@@ -134,6 +141,7 @@ describe('data table', () => {
         .queryAll(By.css('mat-icon'))[0]
         .nativeElement.getAttribute('svgIcon')
     ).toBe('change_history_24px');
+    expect(headerElements[13].nativeElement.innerText).toBe('Smoothed');
   });
 
   it('displays data in order', () => {
@@ -151,6 +159,7 @@ describe('data table', () => {
         ColumnHeaders.MIN_VALUE,
         ColumnHeaders.MAX_VALUE,
         ColumnHeaders.PERCENTAGE_CHANGE,
+        ColumnHeaders.SMOOTHED,
       ],
       data: [
         {
@@ -167,6 +176,7 @@ describe('data table', () => {
           MIN_VALUE: 1,
           MAX_VALUE: 500,
           PERCENTAGE_CHANGE: 0.3,
+          SMOOTHED: 2,
         },
       ],
     });
@@ -199,6 +209,7 @@ describe('data table', () => {
         .queryAll(By.css('mat-icon'))[0]
         .nativeElement.getAttribute('svgIcon')
     ).toBe('arrow_upward_24px');
+    expect(dataElements[13].nativeElement.innerText).toBe('2');
   });
 
   it('displays nothing when no data is available', () => {
@@ -400,4 +411,41 @@ describe('data table', () => {
       ColumnHeaders.RUN,
     ]);
   });
+
+  it('does not display Smoothed column when smoothingEnabled is false', () => {
+    const fixture = createComponent({
+      headers: [
+        ColumnHeaders.VALUE,
+        ColumnHeaders.RUN,
+        ColumnHeaders.SMOOTHED,
+        ColumnHeaders.STEP,
+      ],
+      data: [
+        {
+          id: 'someid',
+          RUN: 'run name',
+          VALUE: 3,
+          STEP: 1,
+          SMOOTHED: 2,
+        },
+      ],
+      smoothingEnabled: false,
+    });
+    fixture.detectChanges();
+    const headerElements = fixture.debugElement.queryAll(By.css('th'));
+    const dataElements = fixture.debugElement.queryAll(By.css('td'));
+
+    // The first header should always be blank as it is the run color column.
+    expect(headerElements[0].nativeElement.innerText).toBe('');
+    expect(headerElements[1].nativeElement.innerText).toBe('Value');
+    expect(headerElements[2].nativeElement.innerText).toBe('Run');
+    expect(headerElements[3].nativeElement.innerText).toBe('Step');
+    expect(headerElements.length).toBe(4);
+
+    expect(dataElements[0].nativeElement.innerText).toBe('');
+    expect(dataElements[1].nativeElement.innerText).toBe('3');
+    expect(dataElements[2].nativeElement.innerText).toBe('run name');
+    expect(dataElements[3].nativeElement.innerText).toBe('1');
+    expect(dataElements.length).toBe(4);
+  });
 });