diff --git a/tensorboard/webapp/feature_flag/store/feature_flag_selectors.ts b/tensorboard/webapp/feature_flag/store/feature_flag_selectors.ts index 8079138386..075a944a45 100644 --- a/tensorboard/webapp/feature_flag/store/feature_flag_selectors.ts +++ b/tensorboard/webapp/feature_flag/store/feature_flag_selectors.ts @@ -91,6 +91,13 @@ export const getEnabledColorGroup = createSelector(getFeatureFlags, (flags) => { return flags.enabledColorGroup; }); +export const getEnabledColorGroupByRegex = createSelector( + getFeatureFlags, + (flags) => { + return flags.enabledColorGroupByRegex; + } +); + export const getIsMetricsImageSupportEnabled = createSelector( getFeatureFlags, (flags) => { diff --git a/tensorboard/webapp/feature_flag/store/feature_flag_selectors_test.ts b/tensorboard/webapp/feature_flag/store/feature_flag_selectors_test.ts index efb2dc09c3..c7d809573a 100644 --- a/tensorboard/webapp/feature_flag/store/feature_flag_selectors_test.ts +++ b/tensorboard/webapp/feature_flag/store/feature_flag_selectors_test.ts @@ -216,6 +216,31 @@ describe('feature_flag_selectors', () => { }); }); + describe('#getEnabledColorGroupByRegex', () => { + it('returns the proper value', () => { + let state = buildState( + buildFeatureFlagState({ + defaultFlags: buildFeatureFlag({ + enabledColorGroupByRegex: false, + }), + }) + ); + expect(selectors.getEnabledColorGroupByRegex(state)).toEqual(false); + + state = buildState( + buildFeatureFlagState({ + defaultFlags: buildFeatureFlag({ + enabledColorGroupByRegex: false, + }), + flagOverrides: { + enabledColorGroupByRegex: true, + }, + }) + ); + expect(selectors.getEnabledColorGroupByRegex(state)).toEqual(true); + }); + }); + describe('#getIsMetricsImageSupportEnabled', () => { it('returns the proper value', () => { let state = buildState( diff --git a/tensorboard/webapp/feature_flag/store/feature_flag_store_config_provider.ts b/tensorboard/webapp/feature_flag/store/feature_flag_store_config_provider.ts index 9131eefc21..c30078ed53 100644 --- a/tensorboard/webapp/feature_flag/store/feature_flag_store_config_provider.ts +++ b/tensorboard/webapp/feature_flag/store/feature_flag_store_config_provider.ts @@ -24,6 +24,7 @@ export const initialState: FeatureFlagState = { defaultEnableDarkMode: false, enableDarkModeOverride: null, enabledColorGroup: false, + enabledColorGroupByRegex: false, enabledExperimentalPlugins: [], inColab: false, scalarsBatchSize: undefined, diff --git a/tensorboard/webapp/feature_flag/testing.ts b/tensorboard/webapp/feature_flag/testing.ts index 1f80fea46b..4dbf5d886a 100644 --- a/tensorboard/webapp/feature_flag/testing.ts +++ b/tensorboard/webapp/feature_flag/testing.ts @@ -23,6 +23,7 @@ export function buildFeatureFlag( defaultEnableDarkMode: false, enableDarkModeOverride: null, enabledColorGroup: false, + enabledColorGroupByRegex: false, enabledExperimentalPlugins: [], inColab: false, scalarsBatchSize: undefined, diff --git a/tensorboard/webapp/feature_flag/types.ts b/tensorboard/webapp/feature_flag/types.ts index 783ebd0cbe..b01e6f644d 100644 --- a/tensorboard/webapp/feature_flag/types.ts +++ b/tensorboard/webapp/feature_flag/types.ts @@ -27,6 +27,8 @@ export interface FeatureFlags { isAutoDarkModeAllowed: boolean; // Whether to enable experimental semantic color grouping feature. enabledColorGroup: boolean; + // Whether to enable color grouping by regex. + enabledColorGroupByRegex: boolean; // experimental plugins to manually enable. enabledExperimentalPlugins: string[]; // Whether the TensorBoard is being run inside Colab output cell. diff --git a/tensorboard/webapp/routes/dashboard_deeplink_provider.ts b/tensorboard/webapp/routes/dashboard_deeplink_provider.ts index 58de9e079e..2d4e1c5bb4 100644 --- a/tensorboard/webapp/routes/dashboard_deeplink_provider.ts +++ b/tensorboard/webapp/routes/dashboard_deeplink_provider.ts @@ -30,6 +30,7 @@ import {GroupBy, GroupByKey} from '../runs/types'; import * as selectors from '../selectors'; import { ENABLE_COLOR_GROUP_QUERY_PARAM_KEY, + ENABLE_COLOR_GROUP_BY_REGEX_QUERY_PARAM_KEY, EXPERIMENTAL_PLUGIN_QUERY_PARAM_KEY, } from '../webapp_data_source/tb_feature_flag_data_source_types'; import { @@ -98,6 +99,14 @@ export class DashboardDeepLinkProvider extends DeepLinkProvider { value: String(overriddenFeatureFlags.enabledColorGroup), }); } + if ( + typeof overriddenFeatureFlags.enabledColorGroupByRegex === 'boolean' + ) { + queryParams.push({ + key: ENABLE_COLOR_GROUP_BY_REGEX_QUERY_PARAM_KEY, + value: String(overriddenFeatureFlags.enabledColorGroupByRegex), + }); + } return queryParams; }) ); diff --git a/tensorboard/webapp/routes/dashboard_deeplink_provider_test.ts b/tensorboard/webapp/routes/dashboard_deeplink_provider_test.ts index 4037a69f26..383d199c34 100644 --- a/tensorboard/webapp/routes/dashboard_deeplink_provider_test.ts +++ b/tensorboard/webapp/routes/dashboard_deeplink_provider_test.ts @@ -347,6 +347,33 @@ describe('core deeplink provider', () => { [] ); }); + + it('serializes enableColorGroupByRegex state', () => { + store.overrideSelector(selectors.getOverriddenFeatureFlags, { + enabledColorGroupByRegex: true, + }); + store.refreshState(); + + expect(queryParamsSerialized[queryParamsSerialized.length - 1]).toEqual([ + {key: 'enableColorGroupByRegex', value: 'true'}, + ]); + + store.overrideSelector(selectors.getOverriddenFeatureFlags, { + enabledColorGroupByRegex: false, + }); + store.refreshState(); + + expect(queryParamsSerialized[queryParamsSerialized.length - 1]).toEqual([ + {key: 'enableColorGroupByRegex', value: 'false'}, + ]); + + store.overrideSelector(selectors.getOverriddenFeatureFlags, {}); + store.refreshState(); + + expect(queryParamsSerialized[queryParamsSerialized.length - 1]).toEqual( + [] + ); + }); }); describe('runs', () => { diff --git a/tensorboard/webapp/runs/views/runs_table/runs_group_menu_button_component.ng.html b/tensorboard/webapp/runs/views/runs_table/runs_group_menu_button_component.ng.html index cda059f6c5..8dcbba31cc 100644 --- a/tensorboard/webapp/runs/views/runs_table/runs_group_menu_button_component.ng.html +++ b/tensorboard/webapp/runs/views/runs_table/runs_group_menu_button_component.ng.html @@ -51,10 +51,10 @@ - diff --git a/tensorboard/webapp/runs/views/runs_table/runs_group_menu_button_component.ts b/tensorboard/webapp/runs/views/runs_table/runs_group_menu_button_component.ts index df92dbfe76..7e416bd18b 100644 --- a/tensorboard/webapp/runs/views/runs_table/runs_group_menu_button_component.ts +++ b/tensorboard/webapp/runs/views/runs_table/runs_group_menu_button_component.ts @@ -31,8 +31,8 @@ import {GroupBy, GroupByKey} from '../../types'; export class RunsGroupMenuButtonComponent { readonly GroupByKey = GroupByKey; - @Input() - selectedGroupBy!: GroupBy; + @Input() selectedGroupBy!: GroupBy; + @Input() showGroupByRegex!: boolean; @Output() onGroupByChange = new EventEmitter(); diff --git a/tensorboard/webapp/runs/views/runs_table/runs_group_menu_button_container.ts b/tensorboard/webapp/runs/views/runs_table/runs_group_menu_button_container.ts index 41cfeabc49..3bdd8f245d 100644 --- a/tensorboard/webapp/runs/views/runs_table/runs_group_menu_button_container.ts +++ b/tensorboard/webapp/runs/views/runs_table/runs_group_menu_button_container.ts @@ -17,6 +17,7 @@ import {Store} from '@ngrx/store'; import {Observable} from 'rxjs'; import {State} from '../../../app_state'; +import {getEnabledColorGroupByRegex} from '../../../selectors'; import {runGroupByChanged} from '../../actions'; import {getRunGroupBy} from '../../store/runs_selectors'; import {GroupBy} from '../../types'; @@ -29,12 +30,17 @@ import {GroupBy} from '../../types'; template: ` `, changeDetection: ChangeDetectionStrategy.OnPush, }) export class RunsGroupMenuButtonContainer { + showGroupByRegex$: Observable = this.store.select( + getEnabledColorGroupByRegex + ); + @Input() experimentIds!: string[]; constructor(private readonly store: Store) {} diff --git a/tensorboard/webapp/runs/views/runs_table/runs_table_test.ts b/tensorboard/webapp/runs/views/runs_table/runs_table_test.ts index e58716b864..7bcb6c8658 100644 --- a/tensorboard/webapp/runs/views/runs_table/runs_table_test.ts +++ b/tensorboard/webapp/runs/views/runs_table/runs_table_test.ts @@ -54,6 +54,7 @@ import {DiscreteFilter, IntervalFilter} from '../../../hparams/types'; import { getCurrentRouteRunSelection, getEnabledColorGroup, + getEnabledColorGroupByRegex, getExperiment, getExperimentIdToAliasMap, getRouteId, @@ -249,6 +250,7 @@ describe('runs_table', () => { ); store.overrideSelector(getRouteId, '123'); store.overrideSelector(getEnabledColorGroup, false); + store.overrideSelector(getEnabledColorGroupByRegex, false); store.overrideSelector(getRunGroupBy, {key: GroupByKey.RUN}); dispatchSpy = spyOn(store, 'dispatch').and.callFake((action: Action) => { actualActions.push(action); @@ -559,9 +561,9 @@ describe('runs_table', () => { expect(menuButton).toBeTruthy(); }); - /* TODO(japie1235813): Brings back group by regex. */ - it('renders "Experiment" and "Run"', () => { + it('renders "Experiment", "Run", and "Regex"', () => { store.overrideSelector(getEnabledColorGroup, true); + store.overrideSelector(getEnabledColorGroupByRegex, true); const fixture = createComponent( ['book'], [RunsTableColumn.RUN_NAME, RunsTableColumn.RUN_COLOR] @@ -577,7 +579,7 @@ describe('runs_table', () => { expect( items.map((element) => element.querySelector('label')!.textContent) - ).toEqual(['Experiment', 'Run']); + ).toEqual(['Experiment', 'Run', 'Regex']); }); it( @@ -585,6 +587,7 @@ describe('runs_table', () => { 'item', () => { store.overrideSelector(getEnabledColorGroup, true); + store.overrideSelector(getEnabledColorGroupByRegex, true); store.overrideSelector(getRunGroupBy, {key: GroupByKey.EXPERIMENT}); const fixture = createComponent( ['book'], @@ -599,15 +602,13 @@ describe('runs_table', () => { const items = getOverlayMenuItems(); - /* TODO(japie1235813): Brings back group by regex. */ expect( items.map((element) => element.getAttribute('aria-checked')) - ).toEqual(['true', 'false']); + ).toEqual(['true', 'false', 'false']); expect( items.map((element) => Boolean(element.querySelector('mat-icon'))) - ).toEqual([true, false]); + ).toEqual([true, false, false]); - /* TODO(japie1235813): Brings back group by regex. store.overrideSelector(getRunGroupBy, { key: GroupByKey.REGEX, regexString: 'hello', @@ -621,12 +622,12 @@ describe('runs_table', () => { expect( items.map((element) => Boolean(element.querySelector('mat-icon'))) ).toEqual([false, false, true]); - */ } ); it('dispatches `runGroupByChanged` when a menu item is clicked', () => { store.overrideSelector(getEnabledColorGroup, true); + store.overrideSelector(getEnabledColorGroupByRegex, true); store.overrideSelector(getRunGroupBy, {key: GroupByKey.EXPERIMENT}); const fixture = createComponent( ['book'], @@ -641,7 +642,7 @@ describe('runs_table', () => { const items = getOverlayMenuItems(); - const [experiments, runs] = items as HTMLElement[]; + const [experiments, runs, regex] = items as HTMLElement[]; experiments.click(); expect(dispatchSpy).toHaveBeenCalledWith( @@ -659,17 +660,15 @@ describe('runs_table', () => { }) ); - /* TODO(japie1235813): Brings back group by regex. regex.click(); expect(dispatchSpy).toHaveBeenCalledWith( runGroupByChanged({ experimentIds: ['book'], - // regexString is hardcoded to '' for now; should be fixed when - // regex support is properly implemented. + // TODO(japie1235813): regexString is hardcoded to '' for now; + // should be fixed when regex support is properly implemented. groupBy: {key: GroupByKey.REGEX, regexString: ''}, }) ); - */ }); it( diff --git a/tensorboard/webapp/webapp_data_source/tb_feature_flag_data_source.ts b/tensorboard/webapp/webapp_data_source/tb_feature_flag_data_source.ts index 7fa094e1f5..5d4d0b8377 100644 --- a/tensorboard/webapp/webapp_data_source/tb_feature_flag_data_source.ts +++ b/tensorboard/webapp/webapp_data_source/tb_feature_flag_data_source.ts @@ -16,6 +16,7 @@ import {Injectable} from '@angular/core'; import { ENABLE_COLOR_GROUP_QUERY_PARAM_KEY, + ENABLE_COLOR_GROUP_BY_REGEX_QUERY_PARAM_KEY, ENABLE_DARK_MODE_QUERY_PARAM_KEY, EXPERIMENTAL_PLUGIN_QUERY_PARAM_KEY, SCALARS_BATCH_SIZE_PARAM_KEY, @@ -68,6 +69,11 @@ export class QueryParamsFeatureFlagDataSource extends TBFeatureFlagDataSource { params.get(ENABLE_COLOR_GROUP_QUERY_PARAM_KEY) !== 'false'; } + if (params.has(ENABLE_COLOR_GROUP_BY_REGEX_QUERY_PARAM_KEY)) { + featureFlags.enabledColorGroupByRegex = + params.get(ENABLE_COLOR_GROUP_BY_REGEX_QUERY_PARAM_KEY) !== 'false'; + } + if (params.has(ENABLE_DARK_MODE_QUERY_PARAM_KEY)) { featureFlags.defaultEnableDarkMode = params.get(ENABLE_DARK_MODE_QUERY_PARAM_KEY) !== 'false'; diff --git a/tensorboard/webapp/webapp_data_source/tb_feature_flag_data_source_test.ts b/tensorboard/webapp/webapp_data_source/tb_feature_flag_data_source_test.ts index 8a125eef66..6a4d1d77fa 100644 --- a/tensorboard/webapp/webapp_data_source/tb_feature_flag_data_source_test.ts +++ b/tensorboard/webapp/webapp_data_source/tb_feature_flag_data_source_test.ts @@ -95,6 +95,27 @@ describe('tb_feature_flag_data_source', () => { expect(dataSource.getFeatures()).toEqual({enabledColorGroup: true}); }); + it('returns enableColorGroupByRegex from the query params', () => { + spyOn(TEST_ONLY.util, 'getParams').and.returnValues( + new URLSearchParams('enableColorGroupByRegex=false'), + new URLSearchParams('enableColorGroupByRegex='), + new URLSearchParams('enableColorGroupByRegex=true'), + new URLSearchParams('enableColorGroupByRegex=foo') + ); + expect(dataSource.getFeatures()).toEqual({ + enabledColorGroupByRegex: false, + }); + expect(dataSource.getFeatures()).toEqual({ + enabledColorGroupByRegex: true, + }); + expect(dataSource.getFeatures()).toEqual({ + enabledColorGroupByRegex: true, + }); + expect(dataSource.getFeatures()).toEqual({ + enabledColorGroupByRegex: true, + }); + }); + it('returns all flag values when they are all set', () => { spyOn(TEST_ONLY.util, 'getParams').and.returnValue( new URLSearchParams( diff --git a/tensorboard/webapp/webapp_data_source/tb_feature_flag_data_source_types.ts b/tensorboard/webapp/webapp_data_source/tb_feature_flag_data_source_types.ts index 033482eb80..ab9967c40c 100644 --- a/tensorboard/webapp/webapp_data_source/tb_feature_flag_data_source_types.ts +++ b/tensorboard/webapp/webapp_data_source/tb_feature_flag_data_source_types.ts @@ -37,4 +37,7 @@ export const SCALARS_BATCH_SIZE_PARAM_KEY = 'scalarsBatchSize'; export const ENABLE_COLOR_GROUP_QUERY_PARAM_KEY = 'enableColorGroup'; +export const ENABLE_COLOR_GROUP_BY_REGEX_QUERY_PARAM_KEY = + 'enableColorGroupByRegex'; + export const ENABLE_DARK_MODE_QUERY_PARAM_KEY = 'darkMode';