timeseries: run selection based on regex filter (#5252)

stephanwlee · web-flow · commit bd2ba99b2419 · 2021-08-18T08:29:33.000-07:00
Previously, regex filter was mainly to influence the list you see on the
runs table. This is where it differs from the traditional runs-selector
(Polymer) which not only filtered runs but also changed the runs
selection that you see on the chart. This change brings the Polymer
behavior to Angular as users have preferred that.
diff --git a/tensorboard/webapp/runs/actions/runs_actions.ts b/tensorboard/webapp/runs/actions/runs_actions.ts
@@ -62,11 +62,6 @@ export const runPageSelectionToggled = createAction(
   props<{experimentIds: string[]; runIds: string[]}>()
 );
 
-export const runsSelectAll = createAction(
-  '[Runs] Runs Select All',
-  props<{experimentIds: string[]}>()
-);
-
 export const runSelectorPaginationOptionChanged = createAction(
   '[Runs] Run Selector Pagination Option Changed',
   props<{pageSize: number; pageIndex: number}>()
diff --git a/tensorboard/webapp/runs/store/runs_reducers.ts b/tensorboard/webapp/runs/store/runs_reducers.ts
@@ -218,24 +218,6 @@ const dataReducer: ActionReducer<RunsDataState, Action> = createReducer(
       selectionState: nextSelectionState,
     };
   }),
-  on(runsActions.runsSelectAll, (state, {experimentIds}) => {
-    const stateKey = serializeExperimentIds(experimentIds);
-    const nextSelectionState = new Map(state.selectionState);
-    const subSelectionState = new Map(nextSelectionState.get(stateKey) ?? []);
-
-    for (const experimentId of experimentIds) {
-      for (const runId of state.runIds[experimentId]) {
-        subSelectionState.set(runId, true);
-      }
-    }
-
-    nextSelectionState.set(stateKey, subSelectionState);
-
-    return {
-      ...state,
-      selectionState: nextSelectionState,
-    };
-  }),
   on(runsActions.fetchRunsSucceeded, (state, {runsForAllExperiments}) => {
     const groupKeyToColorString = new Map(state.groupKeyToColorString);
     const defaultRunColorForGroupBy = new Map(state.defaultRunColorForGroupBy);
diff --git a/tensorboard/webapp/runs/store/runs_reducers_test.ts b/tensorboard/webapp/runs/store/runs_reducers_test.ts
@@ -587,38 +587,6 @@ describe('runs_reducers', () => {
     });
   });
 
-  describe('runsSelectAll', () => {
-    it('selects all runs', () => {
-      const state = buildRunsState({
-        runIds: {
-          e1: ['r1', 'r2'],
-          e2: ['r3'],
-        },
-        selectionState: new Map([['["e1","e2"]', new Map([['r1', false]])]]),
-      });
-
-      const nextState = runsReducers.reducers(
-        state,
-        actions.runsSelectAll({
-          experimentIds: ['e1', 'e2'],
-        })
-      );
-
-      expect(nextState.data.selectionState).toEqual(
-        new Map([
-          [
-            '["e1","e2"]',
-            new Map([
-              ['r1', true],
-              ['r2', true],
-              ['r3', true],
-            ]),
-          ],
-        ])
-      );
-    });
-  });
-
   describe('runSelectorPaginationOptionChanged', () => {
     it('updates the pagination option', () => {
       const state = buildRunsState(undefined, {
diff --git a/tensorboard/webapp/runs/views/runs_table/runs_table_component.ng.html b/tensorboard/webapp/runs/views/runs_table/runs_table_component.ng.html
@@ -24,7 +24,6 @@
 <div class="table-container">
   <div role="table">
     <ng-container *ngTemplateOutlet="header"></ng-container>
-    <ng-container *ngTemplateOutlet="selectAll"></ng-container>
     <div role="rowgroup" class="rows">
       <ng-container *ngFor="let item of pageItems; trackBy: tableTrackBy">
         <ng-container
@@ -58,21 +57,6 @@
   (page)="onPaginationChange.emit($event)"
 ></mat-paginator>
 
-<ng-template #selectAll>
-  <div
-    *ngIf="allPageItemsSelected() && numSelectedItems !== allItemsLength"
-    class="select-all"
-    role="row"
-  >
-    <span role="columnheader"
-      >All runs in this page are selected but not all runs ({{ numSelectedItems
-      }} of {{ allItemsLength }}) are selected.</span
-    ><button mat-button (click)="onSelectAllPages.emit()">
-      Select all runs
-    </button>
-  </div>
-</ng-template>
-
 <ng-template #header>
   <div class="header" role="rowgroup">
     <div
diff --git a/tensorboard/webapp/runs/views/runs_table/runs_table_component.ts b/tensorboard/webapp/runs/views/runs_table/runs_table_component.ts
@@ -109,7 +109,6 @@ export class RunsTableComponent implements OnChanges {
   @Output() onRegexFilterChange = new EventEmitter<string>();
   @Output() onSelectionToggle = new EventEmitter<RunTableItem>();
   @Output() onPageSelectionToggle = new EventEmitter<{items: RunTableItem[]}>();
-  @Output() onSelectAllPages = new EventEmitter<void>();
   @Output()
   onPaginationChange = new EventEmitter<{
     pageIndex: number;
diff --git a/tensorboard/webapp/runs/views/runs_table/runs_table_container.ts b/tensorboard/webapp/runs/views/runs_table/runs_table_container.ts
@@ -67,7 +67,6 @@ import {
   runSelectorPaginationOptionChanged,
   runSelectorRegexFilterChanged,
   runSelectorSortChanged,
-  runsSelectAll,
   runTableShown,
 } from '../../actions';
 import {SortKey, SortType} from '../../types';
@@ -208,7 +207,6 @@ function matchFilter(
       [usePagination]="usePagination"
       (onSelectionToggle)="onRunSelectionToggle($event)"
       (onPageSelectionToggle)="onPageSelectionToggle($event)"
-      (onSelectAllPages)="onSelectAllPages()"
       (onPaginationChange)="onPaginationChange($event)"
       (onRegexFilterChange)="onRegexFilterChange($event)"
       (onSortChange)="onSortChange($event)"
@@ -592,16 +590,6 @@ export class RunsTableContainer implements OnInit, OnDestroy {
     );
   }
 
-  onSelectAllPages() {
-    if (!this.usePagination) {
-      throw new Error(
-        'Select all events cannot be dispatched when pagination is disabled'
-      );
-    }
-
-    this.store.dispatch(runsSelectAll({experimentIds: this.experimentIds}));
-  }
-
   onPaginationChange(event: {pageIndex: number; pageSize: number}) {
     if (!this.usePagination) {
       throw new Error(
diff --git a/tensorboard/webapp/runs/views/runs_table/runs_table_test.ts b/tensorboard/webapp/runs/views/runs_table/runs_table_test.ts
@@ -84,7 +84,6 @@ import {
   runSelectorPaginationOptionChanged,
   runSelectorRegexFilterChanged,
   runSelectorSortChanged,
-  runsSelectAll,
   runTableShown,
 } from '../../actions';
 import {DomainType} from '../../data_source/runs_data_source_types';
@@ -1823,207 +1822,6 @@ describe('runs_table', () => {
         );
       }
     );
-
-    it('does not render select all button when pagination is disabled', () => {
-      store.overrideSelector(getRunSelectorPaginationOption, {
-        pageIndex: 0,
-        pageSize: 2,
-      });
-      store.overrideSelector(getRuns, [
-        buildRun({id: 'book1', name: "The Philosopher's Stone"}),
-        buildRun({id: 'book2', name: 'The Chamber Of Secrets'}),
-        buildRun({id: 'book3', name: 'The Prisoner of Azkaban'}),
-      ]);
-      store.overrideSelector(
-        getCurrentRouteRunSelection,
-        new Map([
-          ['book1', true],
-          ['book2', true],
-          ['book3', false],
-        ])
-      );
-
-      const fixture = createComponent(
-        ['tolkien'],
-        [RunsTableColumn.CHECKBOX, RunsTableColumn.RUN_NAME],
-        false /* usePagination */
-      );
-      fixture.detectChanges();
-
-      const showAll = fixture.nativeElement.querySelector(
-        Selector.SELECT_ALL_ROW
-      );
-      expect(showAll).not.toBeTruthy();
-    });
-
-    it('renders select all button when page is selected but not all items', () => {
-      store.overrideSelector(getRunSelectorPaginationOption, {
-        pageIndex: 0,
-        pageSize: 2,
-      });
-      store.overrideSelector(getRuns, [
-        buildRun({id: 'book1', name: "The Philosopher's Stone"}),
-        buildRun({id: 'book2', name: 'The Chamber Of Secrets'}),
-        buildRun({id: 'book3', name: 'The Prisoner of Azkaban'}),
-      ]);
-      store.overrideSelector(
-        getCurrentRouteRunSelection,
-        new Map([
-          ['book1', true],
-          ['book2', true],
-          ['book3', false],
-        ])
-      );
-
-      const fixture = createComponent(
-        ['tolkien'],
-        [RunsTableColumn.CHECKBOX, RunsTableColumn.RUN_NAME],
-        true /* usePagination */
-      );
-      fixture.detectChanges();
-
-      const showAll = fixture.nativeElement.querySelector(
-        Selector.SELECT_ALL_ROW
-      );
-      expect(showAll.textContent).toContain(
-        'All runs in this page are selected but not all runs (2 of 3)'
-      );
-    });
-
-    it('does not render select if everything is selected', () => {
-      store.overrideSelector(getRunSelectorPaginationOption, {
-        pageIndex: 0,
-        pageSize: 2,
-      });
-      store.overrideSelector(getRuns, [
-        buildRun({id: 'book1', name: "The Philosopher's Stone"}),
-        buildRun({id: 'book2', name: 'The Chamber Of Secrets'}),
-        buildRun({id: 'book3', name: 'The Prisoner of Azkaban'}),
-      ]);
-      store.overrideSelector(
-        getCurrentRouteRunSelection,
-        new Map([
-          ['book1', true],
-          ['book2', true],
-          ['book3', true],
-        ])
-      );
-
-      const fixture = createComponent(
-        ['rowling'],
-        [RunsTableColumn.CHECKBOX, RunsTableColumn.RUN_NAME],
-        true /* usePagination */
-      );
-      fixture.detectChanges();
-
-      const showAll = fixture.nativeElement.querySelector(
-        Selector.SELECT_ALL_ROW
-      );
-      expect(showAll).toBeNull();
-    });
-
-    it('does not render select all if page is not all selected', () => {
-      store.overrideSelector(getRunSelectorPaginationOption, {
-        pageIndex: 0,
-        pageSize: 2,
-      });
-      store.overrideSelector(getRuns, [
-        buildRun({id: 'book1', name: "The Philosopher's Stone"}),
-        buildRun({id: 'book2', name: 'The Chamber Of Secrets'}),
-        buildRun({id: 'book3', name: 'The Prisoner of Azkaban'}),
-      ]);
-      store.overrideSelector(
-        getCurrentRouteRunSelection,
-        new Map([
-          ['book1', true],
-          ['book2', false],
-          ['book3', true],
-        ])
-      );
-
-      const fixture = createComponent(
-        ['rowling'],
-        [RunsTableColumn.CHECKBOX, RunsTableColumn.RUN_NAME],
-        true /* usePagination */
-      );
-      fixture.detectChanges();
-
-      const showAll = fixture.nativeElement.querySelector(
-        Selector.SELECT_ALL_ROW
-      );
-      expect(showAll).toBeNull();
-    });
-
-    it('renders select all even when all filtered items are selected', () => {
-      store.overrideSelector(getRunSelectorRegexFilter, '[oO]f');
-      store.overrideSelector(getRunSelectorPaginationOption, {
-        pageIndex: 0,
-        pageSize: 2,
-      });
-      store.overrideSelector(getRuns, [
-        buildRun({id: 'book1', name: "The Philosopher's Stone"}),
-        buildRun({id: 'book2', name: 'The Chamber Of Secrets'}),
-        buildRun({id: 'book3', name: 'The Prisoner of Azkaban'}),
-      ]);
-      store.overrideSelector(
-        getCurrentRouteRunSelection,
-        new Map([
-          ['book1', false],
-          ['book2', true],
-          ['book3', true],
-        ])
-      );
-
-      const fixture = createComponent(
-        ['tolkien'],
-        [RunsTableColumn.CHECKBOX, RunsTableColumn.RUN_NAME],
-        true /* usePagination */
-      );
-      fixture.detectChanges();
-
-      const showAll = fixture.nativeElement.querySelector(
-        Selector.SELECT_ALL_ROW
-      );
-      expect(showAll.textContent).toContain(
-        'All runs in this page are selected but not all runs (2 of 3)'
-      );
-    });
-
-    it('dispatches runsSelectAll when click on select', () => {
-      store.overrideSelector(getRunSelectorPaginationOption, {
-        pageIndex: 0,
-        pageSize: 2,
-      });
-      store.overrideSelector(getRuns, [
-        buildRun({id: 'book1', name: "The Philosopher's Stone"}),
-        buildRun({id: 'book2', name: 'The Chamber Of Secrets'}),
-        buildRun({id: 'book3', name: 'The Prisoner of Azkaban'}),
-      ]);
-      store.overrideSelector(
-        getCurrentRouteRunSelection,
-        new Map([
-          ['book1', true],
-          ['book2', true],
-          ['book3', false],
-        ])
-      );
-
-      const fixture = createComponent(
-        ['rowling'],
-        [RunsTableColumn.CHECKBOX, RunsTableColumn.RUN_NAME],
-        true /* usePagination */
-      );
-      fixture.detectChanges();
-
-      const button = fixture.nativeElement.querySelector('.select-all button');
-      button.click();
-
-      expect(dispatchSpy).toHaveBeenCalledWith(
-        runsSelectAll({
-          experimentIds: ['rowling'],
-        })
-      );
-    });
   });
 
   describe('"too many runs" alert', () => {
diff --git a/tensorboard/webapp/util/ui_selectors.ts b/tensorboard/webapp/util/ui_selectors.ts
@@ -29,7 +29,10 @@ import {createSelector} from '@ngrx/store';
 
 import {getExperimentIdsFromRoute} from '../app_routing/store/app_routing_selectors';
 import {State} from '../app_state';
-import {getRunSelectionMap} from '../runs/store/runs_selectors';
+import {
+  getRunSelectionMap,
+  getRunSelectorRegexFilter,
+} from '../runs/store/runs_selectors';
 
 /** @typehack */ import * as _typeHackStore from '@ngrx/store';
 
@@ -46,5 +49,22 @@ export const getCurrentRouteRunSelection = createSelector(
     }
     return getRunSelectionMap(state, {experimentIds});
   },
-  (runSelection) => runSelection
+  getRunSelectorRegexFilter,
+  (runSelection, regexFilter) => {
+    if (!runSelection) return null;
+
+    let regexExp: RegExp;
+
+    try {
+      regexExp = new RegExp(regexFilter, 'i');
+    } catch {
+      regexExp = new RegExp('');
+    }
+
+    const filteredSelection = new Map<string, boolean>();
+    for (const [key, value] of runSelection.entries()) {
+      filteredSelection.set(key, regexExp.test(key) && value);
+    }
+    return filteredSelection;
+  }
 );
diff --git a/tensorboard/webapp/util/ui_selectors_test.ts b/tensorboard/webapp/util/ui_selectors_test.ts
