Skip to content

Commit 457a0ab

Browse files
JamesHollyerJamesHollyer
authored
Step Selector: Persist step selection settings. (#5975)
* Motivation for features / changes The new step selection feature currently must be enabled every time you load Tensorboard. This is annoying for users who like this feature and have to turn it on constantly. This change persists the settings in LocalStorage so it can stay enabled on that machine until it is disabled. * Technical description of changes Just followed the pattern of settings that are already persisted.
1 parent b336632 commit 457a0ab

File tree

6 files changed

+234
-0
lines changed

6 files changed

+234
-0
lines changed

‎tensorboard/webapp/metrics/metrics_module.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,10 @@ import {MetricsEffects} from './effects';
2727
import {
2828
getMetricsCardMinWidth,
2929
getMetricsIgnoreOutliers,
30+
getMetricsLinkedTimeEnabled,
31+
getMetricsRangeSelectionEnabled,
3032
getMetricsScalarSmoothing,
33+
getMetricsStepSelectorEnabled,
3134
getMetricsTooltipSort,
3235
isMetricsSettingsPaneOpen,
3336
METRICS_FEATURE_KEY,
@@ -95,6 +98,22 @@ export function getMetricsTimeSeriesCardMinWidth() {
9598
return {timeSeriesCardMinWidth: cardMinWidth};
9699
});
97100
}
101+
export function getMetricsTimeSeriesStepSelectorEnabled() {
102+
return createSelector(getMetricsStepSelectorEnabled, (isEnabled) => {
103+
return {stepSelectorEnabled: isEnabled};
104+
});
105+
}
106+
107+
export function getMetricsTimeSeriesRangeSelectionEnabled() {
108+
return createSelector(getMetricsRangeSelectionEnabled, (isEnabled) => {
109+
return {rangeSelectionEnabled: isEnabled};
110+
});
111+
}
112+
export function getMetricsTimeSeriesLinkedTimeEnabled() {
113+
return createSelector(getMetricsLinkedTimeEnabled, (isEnabled) => {
114+
return {linkedTimeEnabled: isEnabled};
115+
});
116+
}
98117

99118
@NgModule({
100119
imports: [
@@ -129,6 +148,15 @@ export function getMetricsTimeSeriesCardMinWidth() {
129148
PersistentSettingsConfigModule.defineGlobalSetting(
130149
getMetricsTimeSeriesCardMinWidth
131150
),
151+
PersistentSettingsConfigModule.defineGlobalSetting(
152+
getMetricsTimeSeriesStepSelectorEnabled
153+
),
154+
PersistentSettingsConfigModule.defineGlobalSetting(
155+
getMetricsTimeSeriesRangeSelectionEnabled
156+
),
157+
PersistentSettingsConfigModule.defineGlobalSetting(
158+
getMetricsTimeSeriesLinkedTimeEnabled
159+
),
132160
],
133161
providers: [
134162
{

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -436,10 +436,19 @@ const reducer = createReducer(
436436

437437
const isSettingsPaneOpen =
438438
partialSettings.timeSeriesSettingsPaneOpened ?? state.isSettingsPaneOpen;
439+
const stepSelectorEnabled =
440+
partialSettings.stepSelectorEnabled ?? state.stepSelectorEnabled;
441+
const rangeSelectionEnabled =
442+
partialSettings.rangeSelectionEnabled ?? state.rangeSelectionEnabled;
443+
const linkedTimeEnabled =
444+
partialSettings.linkedTimeEnabled ?? state.linkedTimeEnabled;
439445

440446
return {
441447
...state,
442448
isSettingsPaneOpen,
449+
stepSelectorEnabled,
450+
rangeSelectionEnabled,
451+
linkedTimeEnabled,
443452
settings: {
444453
...state.settings,
445454
...metricsSettings,

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

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2414,6 +2414,69 @@ describe('metrics reducers', () => {
24142414
});
24152415
});
24162416

2417+
it('loads Step Selector Setting into the next state', () => {
2418+
const beforeState = buildMetricsState({
2419+
stepSelectorEnabled: false,
2420+
rangeSelectionEnabled: false,
2421+
linkedTimeEnabled: false,
2422+
});
2423+
2424+
const nextState = reducers(
2425+
beforeState,
2426+
globalSettingsLoaded({
2427+
partialSettings: {
2428+
stepSelectorEnabled: true,
2429+
},
2430+
})
2431+
);
2432+
2433+
expect(nextState.stepSelectorEnabled).toBe(true);
2434+
expect(nextState.rangeSelectionEnabled).toBe(false);
2435+
expect(nextState.linkedTimeEnabled).toBe(false);
2436+
});
2437+
2438+
it('loads Step Selector Setting into the next state', () => {
2439+
const beforeState = buildMetricsState({
2440+
stepSelectorEnabled: false,
2441+
rangeSelectionEnabled: false,
2442+
linkedTimeEnabled: false,
2443+
});
2444+
2445+
const nextState = reducers(
2446+
beforeState,
2447+
globalSettingsLoaded({
2448+
partialSettings: {
2449+
rangeSelectionEnabled: true,
2450+
},
2451+
})
2452+
);
2453+
2454+
expect(nextState.stepSelectorEnabled).toBe(false);
2455+
expect(nextState.rangeSelectionEnabled).toBe(true);
2456+
expect(nextState.linkedTimeEnabled).toBe(false);
2457+
});
2458+
2459+
it('loads Step Selector Setting into the next state', () => {
2460+
const beforeState = buildMetricsState({
2461+
stepSelectorEnabled: false,
2462+
rangeSelectionEnabled: false,
2463+
linkedTimeEnabled: false,
2464+
});
2465+
2466+
const nextState = reducers(
2467+
beforeState,
2468+
globalSettingsLoaded({
2469+
partialSettings: {
2470+
linkedTimeEnabled: true,
2471+
},
2472+
})
2473+
);
2474+
2475+
expect(nextState.stepSelectorEnabled).toBe(false);
2476+
expect(nextState.rangeSelectionEnabled).toBe(false);
2477+
expect(nextState.linkedTimeEnabled).toBe(true);
2478+
});
2479+
24172480
describe('linked time features', () => {
24182481
describe('#timeSelectionChanged', () => {
24192482
const imageCardId = 'test image card id "plugin":"images"';

‎tensorboard/webapp/persistent_settings/_data_source/persistent_settings_data_source.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,16 @@ export class OSSSettingsConverter extends SettingsConverter<
8484
serializableSettings.timeSeriesCardMinWidth =
8585
settings.timeSeriesCardMinWidth;
8686
}
87+
if (settings.stepSelectorEnabled !== undefined) {
88+
serializableSettings.stepSelectorEnabled = settings.stepSelectorEnabled;
89+
}
90+
if (settings.rangeSelectionEnabled !== undefined) {
91+
serializableSettings.rangeSelectionEnabled =
92+
settings.rangeSelectionEnabled;
93+
}
94+
if (settings.linkedTimeEnabled !== undefined) {
95+
serializableSettings.linkedTimeEnabled = settings.linkedTimeEnabled;
96+
}
8797
return serializableSettings;
8898
}
8999

@@ -169,6 +179,27 @@ export class OSSSettingsConverter extends SettingsConverter<
169179
settings.timeSeriesCardMinWidth = backendSettings.timeSeriesCardMinWidth;
170180
}
171181

182+
if (
183+
backendSettings.hasOwnProperty('stepSelectorEnabled') &&
184+
typeof backendSettings.stepSelectorEnabled === 'boolean'
185+
) {
186+
settings.stepSelectorEnabled = backendSettings.stepSelectorEnabled;
187+
}
188+
189+
if (
190+
backendSettings.hasOwnProperty('rangeSelectionEnabled') &&
191+
typeof backendSettings.rangeSelectionEnabled === 'boolean'
192+
) {
193+
settings.rangeSelectionEnabled = backendSettings.rangeSelectionEnabled;
194+
}
195+
196+
if (
197+
backendSettings.hasOwnProperty('linkedTimeEnabled') &&
198+
typeof backendSettings.linkedTimeEnabled === 'boolean'
199+
) {
200+
settings.linkedTimeEnabled = backendSettings.linkedTimeEnabled;
201+
}
202+
172203
return settings;
173204
}
174205
}

‎tensorboard/webapp/persistent_settings/_data_source/persistent_settings_data_source_test.ts

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,46 @@ describe('persistent_settings data_source test', () => {
141141
themeOverride: ThemeValue.DARK,
142142
});
143143
});
144+
145+
it('properly converts stepSelectorEnabled', async () => {
146+
getItemSpy.withArgs(TEST_ONLY.GLOBAL_LOCAL_STORAGE_KEY).and.returnValue(
147+
JSON.stringify({
148+
stepSelectorEnabled: true,
149+
})
150+
);
151+
152+
const actual = await firstValueFrom(dataSource.getSettings());
153+
154+
expect(actual).toEqual({
155+
stepSelectorEnabled: true,
156+
});
157+
});
158+
it('properly converts rangeSelectionEnabled', async () => {
159+
getItemSpy.withArgs(TEST_ONLY.GLOBAL_LOCAL_STORAGE_KEY).and.returnValue(
160+
JSON.stringify({
161+
rangeSelectionEnabled: true,
162+
})
163+
);
164+
165+
const actual = await firstValueFrom(dataSource.getSettings());
166+
167+
expect(actual).toEqual({
168+
rangeSelectionEnabled: true,
169+
});
170+
});
171+
it('properly converts linkedTimeEnabled', async () => {
172+
getItemSpy.withArgs(TEST_ONLY.GLOBAL_LOCAL_STORAGE_KEY).and.returnValue(
173+
JSON.stringify({
174+
linkedTimeEnabled: true,
175+
})
176+
);
177+
178+
const actual = await firstValueFrom(dataSource.getSettings());
179+
180+
expect(actual).toEqual({
181+
linkedTimeEnabled: true,
182+
});
183+
});
144184
});
145185

146186
describe('#setSettings', () => {
@@ -167,6 +207,63 @@ describe('persistent_settings data_source test', () => {
167207
})
168208
);
169209
});
210+
211+
it('properly converts stepSelectorEnabled', async () => {
212+
getItemSpy
213+
.withArgs(TEST_ONLY.GLOBAL_LOCAL_STORAGE_KEY)
214+
.and.returnValue(null);
215+
216+
await firstValueFrom(
217+
dataSource.setSettings({
218+
stepSelectorEnabled: true,
219+
})
220+
);
221+
222+
expect(setItemSpy).toHaveBeenCalledOnceWith(
223+
TEST_ONLY.GLOBAL_LOCAL_STORAGE_KEY,
224+
JSON.stringify({
225+
stepSelectorEnabled: true,
226+
})
227+
);
228+
});
229+
230+
it('properly converts rangeSelectionEnabled', async () => {
231+
getItemSpy
232+
.withArgs(TEST_ONLY.GLOBAL_LOCAL_STORAGE_KEY)
233+
.and.returnValue(null);
234+
235+
await firstValueFrom(
236+
dataSource.setSettings({
237+
rangeSelectionEnabled: true,
238+
})
239+
);
240+
241+
expect(setItemSpy).toHaveBeenCalledOnceWith(
242+
TEST_ONLY.GLOBAL_LOCAL_STORAGE_KEY,
243+
JSON.stringify({
244+
rangeSelectionEnabled: true,
245+
})
246+
);
247+
});
248+
249+
it('properly converts linkedTimeEnabled', async () => {
250+
getItemSpy
251+
.withArgs(TEST_ONLY.GLOBAL_LOCAL_STORAGE_KEY)
252+
.and.returnValue(null);
253+
254+
await firstValueFrom(
255+
dataSource.setSettings({
256+
linkedTimeEnabled: true,
257+
})
258+
);
259+
260+
expect(setItemSpy).toHaveBeenCalledOnceWith(
261+
TEST_ONLY.GLOBAL_LOCAL_STORAGE_KEY,
262+
JSON.stringify({
263+
linkedTimeEnabled: true,
264+
})
265+
);
266+
});
170267
});
171268

172269
describe('settings migration', () => {

‎tensorboard/webapp/persistent_settings/_data_source/types.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,9 @@ export declare interface BackendSettings {
3838
sideBarWidthInPercent?: number;
3939
timeSeriesSettingsPaneOpened?: boolean;
4040
timeSeriesCardMinWidth?: number;
41+
stepSelectorEnabled?: boolean;
42+
rangeSelectionEnabled?: boolean;
43+
linkedTimeEnabled?: boolean;
4144
}
4245

4346
/**
@@ -57,4 +60,7 @@ export interface PersistableSettings {
5760
sideBarWidthInPercent?: number;
5861
timeSeriesSettingsPaneOpened?: boolean;
5962
timeSeriesCardMinWidth?: number;
63+
stepSelectorEnabled?: boolean;
64+
rangeSelectionEnabled?: boolean;
65+
linkedTimeEnabled?: boolean;
6066
}

0 commit comments

Comments
 (0)