Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Time Series: preserve run regex filter string in URL #5412

Merged
merged 5 commits into from
Nov 12, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions tensorboard/webapp/routes/dashboard_deeplink_provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import {
RUN_COLOR_GROUP_KEY,
DeserializedState,
PINNED_CARDS_KEY,
RUN_FILTER_KEY,
SMOOTHING_KEY,
TAG_FILTER_KEY,
} from './dashboard_deeplink_provider_types';
Expand Down Expand Up @@ -162,6 +163,12 @@ export class DashboardDeepLinkProvider extends DeepLinkProvider {
return [{key: RUN_COLOR_GROUP_KEY, value}];
})
),
store.select(selectors.getRunSelectorRegexFilter).pipe(
map((value) => {
if (!value) return [];
return [{key: RUN_FILTER_KEY, value}];
})
),
]).pipe(
map((queryParamList) => {
return queryParamList.flat();
Expand All @@ -176,6 +183,7 @@ export class DashboardDeepLinkProvider extends DeepLinkProvider {
let smoothing = null;
let tagFilter = null;
let groupBy: GroupBy | null = null;
let runFilter = null;

for (const {key, value} of queryParams) {
switch (key) {
Expand Down Expand Up @@ -206,6 +214,9 @@ export class DashboardDeepLinkProvider extends DeepLinkProvider {
case TAG_FILTER_KEY:
tagFilter = value;
break;
case RUN_FILTER_KEY:
runFilter = value;
break;
}
}

Expand All @@ -217,6 +228,7 @@ export class DashboardDeepLinkProvider extends DeepLinkProvider {
},
runs: {
groupBy,
regexFilter: runFilter,
},
};
}
Expand Down
133 changes: 84 additions & 49 deletions tensorboard/webapp/routes/dashboard_deeplink_provider_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ describe('core deeplink provider', () => {
store.overrideSelector(selectors.getOverriddenFeatureFlags, {});
store.overrideSelector(selectors.getMetricsSettingOverrides, {});
store.overrideSelector(selectors.getRunUserSetGroupBy, null);
store.overrideSelector(selectors.getRunSelectorRegexFilter, '');

queryParamsSerialized = [];

Expand Down Expand Up @@ -419,67 +420,101 @@ describe('core deeplink provider', () => {
});

describe('runs', () => {
it('does not put state in the URL when user set color group is null', () => {
// Setting from `null` to `null` does not actually trigger the provider so
// we have to set it: `null` -> something else -> `null` to test this
// case.
store.overrideSelector(selectors.getRunUserSetGroupBy, {
key: GroupByKey.EXPERIMENT,
describe('color group', () => {
it('does not put state in the URL when user set color group is null', () => {
// Setting from `null` to `null` does not actually trigger the provider so
// we have to set it: `null` -> something else -> `null` to test this
// case.
store.overrideSelector(selectors.getRunUserSetGroupBy, {
key: GroupByKey.EXPERIMENT,
});
store.refreshState();

store.overrideSelector(selectors.getRunUserSetGroupBy, null);
store.refreshState();
expect(queryParamsSerialized[queryParamsSerialized.length - 1]).toEqual(
[]
);
});
store.refreshState();

store.overrideSelector(selectors.getRunUserSetGroupBy, null);
store.refreshState();
expect(queryParamsSerialized[queryParamsSerialized.length - 1]).toEqual(
[]
);
});
it('serializes user set color group settings', () => {
store.overrideSelector(selectors.getRunUserSetGroupBy, {
key: GroupByKey.EXPERIMENT,
});
store.refreshState();
expect(queryParamsSerialized[queryParamsSerialized.length - 1]).toEqual(
[{key: 'runColorGroup', value: 'experiment'}]
);

it('serializes user set color group settings', () => {
store.overrideSelector(selectors.getRunUserSetGroupBy, {
key: GroupByKey.EXPERIMENT,
});
store.refreshState();
expect(queryParamsSerialized[queryParamsSerialized.length - 1]).toEqual([
{key: 'runColorGroup', value: 'experiment'},
]);
store.overrideSelector(selectors.getRunUserSetGroupBy, {
key: GroupByKey.RUN,
});
store.refreshState();
expect(queryParamsSerialized[queryParamsSerialized.length - 1]).toEqual(
[{key: 'runColorGroup', value: 'run'}]
);

store.overrideSelector(selectors.getRunUserSetGroupBy, {
key: GroupByKey.RUN,
store.overrideSelector(selectors.getRunUserSetGroupBy, {
key: GroupByKey.REGEX,
regexString: 'hello:world',
});
store.refreshState();
expect(queryParamsSerialized[queryParamsSerialized.length - 1]).toEqual(
[{key: 'runColorGroup', value: 'regex:hello:world'}]
);
});
store.refreshState();
expect(queryParamsSerialized[queryParamsSerialized.length - 1]).toEqual([
{key: 'runColorGroup', value: 'run'},
]);

store.overrideSelector(selectors.getRunUserSetGroupBy, {
key: GroupByKey.REGEX,
regexString: 'hello:world',
it('serializes interesting regex strings', () => {
store.overrideSelector(selectors.getRunUserSetGroupBy, {
key: GroupByKey.REGEX,
regexString: '',
});
store.refreshState();
expect(queryParamsSerialized[queryParamsSerialized.length - 1]).toEqual(
[{key: 'runColorGroup', value: 'regex:'}]
);

store.overrideSelector(selectors.getRunUserSetGroupBy, {
key: GroupByKey.REGEX,
regexString: 'hello/(world):goodbye',
});
store.refreshState();
expect(queryParamsSerialized[queryParamsSerialized.length - 1]).toEqual(
[{key: 'runColorGroup', value: 'regex:hello/(world):goodbye'}]
);
});
store.refreshState();
expect(queryParamsSerialized[queryParamsSerialized.length - 1]).toEqual([
{key: 'runColorGroup', value: 'regex:hello:world'},
]);
});

it('serializes interesting regex strings', () => {
store.overrideSelector(selectors.getRunUserSetGroupBy, {
key: GroupByKey.REGEX,
regexString: '',
describe('filter', () => {
it('does not serialize an empty string', () => {
store.overrideSelector(selectors.getRunSelectorRegexFilter, '');
store.refreshState();

expect(queryParamsSerialized).toEqual([]);
});
store.refreshState();
expect(queryParamsSerialized[queryParamsSerialized.length - 1]).toEqual([
{key: 'runColorGroup', value: 'regex:'},
]);

store.overrideSelector(selectors.getRunUserSetGroupBy, {
key: GroupByKey.REGEX,
regexString: 'hello/(world):goodbye',
it('serializes runFilter state', () => {
store.overrideSelector(selectors.getRunSelectorRegexFilter, 'hello');
store.refreshState();

expect(queryParamsSerialized[queryParamsSerialized.length - 1]).toEqual(
[{key: 'runFilter', value: 'hello'}]
);

store.overrideSelector(selectors.getRunSelectorRegexFilter, 'hello:');
store.refreshState();

expect(queryParamsSerialized[queryParamsSerialized.length - 1]).toEqual(
[{key: 'runFilter', value: 'hello:'}]
);

store.overrideSelector(selectors.getRunSelectorRegexFilter, 'hello:.*');
store.refreshState();

expect(queryParamsSerialized[queryParamsSerialized.length - 1]).toEqual(
[{key: 'runFilter', value: 'hello:.*'}]
);
});
store.refreshState();
expect(queryParamsSerialized[queryParamsSerialized.length - 1]).toEqual([
{key: 'runColorGroup', value: 'regex:hello/(world):goodbye'},
]);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,5 @@ export const PINNED_CARDS_KEY = 'pinnedCards';
export const RUN_COLOR_GROUP_KEY = 'runColorGroup';

export const TAG_FILTER_KEY = 'tagFilter';

export const RUN_FILTER_KEY = 'runFilter';
1 change: 1 addition & 0 deletions tensorboard/webapp/routes/testing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export function buildDeserializedState(
return {
runs: {
groupBy: null,
regexFilter: null,
},
metrics: {
pinnedCards: [],
Expand Down
19 changes: 12 additions & 7 deletions tensorboard/webapp/runs/store/runs_reducers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,19 +97,24 @@ const dataReducer: ActionReducer<RunsDataState, Action> = createReducer(

const dehydratedState = partialState as URLDeserializedState;
const groupBy = dehydratedState.runs.groupBy;
if (!groupBy) {
const regexFilter = dehydratedState.runs.regexFilter ?? '';

if (!groupBy && !regexFilter) {
return state;
}

const regexString =
groupBy.key === GroupByKey.REGEX
? groupBy.regexString
: state.colorGroupRegexString;
if (groupBy) {
const regexString =
groupBy.key === GroupByKey.REGEX
? groupBy.regexString
: state.colorGroupRegexString;
state.colorGroupRegexString = regexString;
state.userSetGroupByKey = groupBy.key ?? null;
}

return {
...state,
colorGroupRegexString: regexString,
userSetGroupByKey: groupBy.key ?? null,
regexFilter,
};
}),
on(runsActions.fetchRunsRequested, (state, action) => {
Expand Down
75 changes: 75 additions & 0 deletions tensorboard/webapp/runs/store/runs_reducers_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1009,6 +1009,7 @@ describe('runs_reducers', () => {
const partialState: URLDeserializedState = {
runs: {
groupBy: {key: GroupByKey.EXPERIMENT},
regexFilter: null,
},
};
const nextState = runsReducers.reducers(
Expand All @@ -1031,6 +1032,7 @@ describe('runs_reducers', () => {
const partialState: URLDeserializedState = {
runs: {
groupBy: null,
regexFilter: null,
},
};
const nextState = runsReducers.reducers(
Expand Down Expand Up @@ -1082,6 +1084,7 @@ describe('runs_reducers', () => {
const partialState: URLDeserializedState = {
runs: {
groupBy: {key: GroupByKey.EXPERIMENT},
regexFilter: null,
},
};
const nextState = runsReducers.reducers(
Expand Down Expand Up @@ -1118,6 +1121,7 @@ describe('runs_reducers', () => {
const partialState: URLDeserializedState = {
runs: {
groupBy: {key: GroupByKey.REGEX, regexString: 'regex string'},
regexFilter: null,
},
};
const nextState = runsReducers.reducers(
Expand All @@ -1130,6 +1134,77 @@ describe('runs_reducers', () => {

expect(nextState.data.colorGroupRegexString).toBe('regex string');
});

it('does not set regexFilter when null value provided', () => {
const state = buildRunsState({
regexFilter: 'hello',
});

const partialState: URLDeserializedState = {
runs: {
groupBy: null,
regexFilter: null,
},
};
const nextState = runsReducers.reducers(
state,
stateRehydratedFromUrl({
routeKind: RouteKind.EXPERIMENT,
partialState,
})
);

expect(nextState.data.regexFilter).toBe('hello');
});

it('sets regexFilter to the valid value provided', () => {
const state = buildRunsState({
regexFilter: 'hello',
});

const partialState: URLDeserializedState = {
runs: {
groupBy: null,
regexFilter: 'world',
},
};
const nextState = runsReducers.reducers(
state,
stateRehydratedFromUrl({
routeKind: RouteKind.EXPERIMENT,
partialState,
})
);

expect(nextState.data.regexFilter).toBe('world');
});

it('set regexFilter and userSetGroupBy to the value provided', () => {
const state = buildRunsState({
colorGroupRegexString: '',
initialGroupBy: {key: GroupByKey.REGEX, regexString: ''},
userSetGroupByKey: GroupByKey.RUN,
regexFilter: 'hello',
});

const partialState: URLDeserializedState = {
runs: {
groupBy: {key: GroupByKey.EXPERIMENT},
regexFilter: 'world',
},
};

const nextState = runsReducers.reducers(
state,
stateRehydratedFromUrl({
routeKind: RouteKind.EXPERIMENT,
partialState,
})
);

expect(nextState.data.regexFilter).toBe('world');
expect(nextState.data.userSetGroupByKey).toBe(GroupByKey.EXPERIMENT);
});
});

describe('when freshly navigating', () => {
Expand Down
1 change: 1 addition & 0 deletions tensorboard/webapp/runs/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,5 +78,6 @@ export type GroupBy = BaseGroupBy | RegexGroupBy;
export interface URLDeserializedState {
runs: {
groupBy: GroupBy | null;
regexFilter: string | null;
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
<div class="filter-row">
<tb-filter-input
class="run-filter"
value="{{ regexFilter }}"
(keyup)="onFilterKeyUp($event)"
placeholder="Filter runs (regex)"
></tb-filter-input>
Expand Down