Skip to content

Commit 4db76ed

Browse files
mattseddonmattseddon
authored
Decorate filtered experiments/checkpoints in the experiments tree (#1871)
* add git ignore decoration to filtered experiments in the experiments tree * exclude queued from filter counts * add variable for readability * destructure filtered accumulator * rename filter experiment function
1 parent 70d345a commit 4db76ed

File tree

12 files changed

+266
-131
lines changed

12 files changed

+266
-131
lines changed

‎extension/src/__mocks__/vscode.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ export enum TreeItemCollapsibleState {
2020
}
2121
export const Uri = {
2222
file: URI.file,
23+
from: URI.from,
2324
joinPath: Utils.joinPath
2425
}
2526
export const window = {

‎extension/src/experiments/index.ts‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import { CheckpointsModel } from './checkpoints/model'
1515
import { ExperimentsData } from './data'
1616
import { askToDisableAutoApplyFilters } from './toast'
1717
import { Experiment, ColumnType, TableData } from './webview/contract'
18+
import { DecorationProvider } from './model/filterBy/decorationProvider'
1819
import { SortDefinition } from './model/sortBy'
1920
import { splitColumnPath } from './columns/paths'
2021
import { ResourceLocator } from '../resourceLocator'
@@ -74,6 +75,9 @@ export class Experiments extends BaseRepository<TableData> {
7475
)
7576

7677
private readonly columnsChanged = this.dispose.track(new EventEmitter<void>())
78+
private readonly decorationProvider = this.dispose.track(
79+
new DecorationProvider()
80+
)
7781

7882
private readonly internalCommands: InternalCommands
7983

@@ -257,8 +261,8 @@ export class Experiments extends BaseRepository<TableData> {
257261
return this.notifyChanged()
258262
}
259263

260-
public getFilteredCounts() {
261-
return this.experiments.getFilteredCounts()
264+
public getFilteredExperiments() {
265+
return this.experiments.getFilteredExperiments()
262266
}
263267

264268
public getExperimentCount() {
@@ -295,7 +299,7 @@ export class Experiments extends BaseRepository<TableData> {
295299

296300
if (useFilters) {
297301
const filteredExperiments = this.experiments
298-
.getFilteredExperiments()
302+
.getUnfilteredExperiments()
299303
.filter(exp => !exp.queued)
300304
if (tooManySelected(filteredExperiments)) {
301305
await this.warnAndDoNotAutoApply(filteredExperiments)
@@ -425,6 +429,10 @@ export class Experiments extends BaseRepository<TableData> {
425429
}
426430

427431
private notifyChanged(data?: ExperimentsOutput) {
432+
this.decorationProvider.setState(
433+
this.experiments.getLabels(),
434+
this.experiments.getLabelsToDecorate()
435+
)
428436
this.experimentsChanged.fire(data)
429437
this.notifyColumnsChanged()
430438
}

‎extension/src/experiments/model/filterBy/collect.ts‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
import {
2+
FilterDefinition,
3+
filterExperiment,
4+
splitExperimentsByFilters
5+
} from '.'
6+
import { ExperimentType } from '..'
7+
import { definedAndNonEmpty } from '../../../util/array'
8+
import { Experiment } from '../../webview/contract'
9+
10+
export type ExperimentWithType = Experiment & { type: ExperimentType }
11+
12+
export const collectFilteredCounts = (
13+
experiments: { type: ExperimentType }[]
14+
) => {
15+
const filtered = { checkpoints: 0, experiments: 0 }
16+
17+
for (const { type } of experiments) {
18+
if (type === ExperimentType.CHECKPOINT) {
19+
filtered.checkpoints = filtered.checkpoints + 1
20+
}
21+
if (type === ExperimentType.EXPERIMENT) {
22+
filtered.experiments = filtered.experiments + 1
23+
}
24+
}
25+
26+
return filtered
27+
}
28+
29+
export const collectFiltered = (
30+
acc: ExperimentWithType[],
31+
filters: FilterDefinition[],
32+
experiment: Experiment,
33+
checkpoints: ExperimentWithType[]
34+
): ExperimentWithType[] => {
35+
const { filtered, unfiltered } = splitExperimentsByFilters(
36+
filters,
37+
checkpoints
38+
)
39+
acc.push(...filtered)
40+
const hasUnfilteredCheckpoints = definedAndNonEmpty(unfiltered)
41+
if (hasUnfilteredCheckpoints) {
42+
return acc
43+
}
44+
if (!filterExperiment(filters, experiment)) {
45+
acc.push({ ...experiment, type: ExperimentType.EXPERIMENT })
46+
}
47+
return acc
48+
}

‎extension/src/experiments/model/filterBy/decorationProvider.ts‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
import {
2+
Event,
3+
EventEmitter,
4+
FileDecoration,
5+
FileDecorationProvider,
6+
ThemeColor,
7+
Uri,
8+
window
9+
} from 'vscode'
10+
import { Disposable } from '../../../class/dispose'
11+
12+
export const getDecoratableUri = (label: string): Uri =>
13+
Uri.from({ path: label, scheme: 'dvc.experiments' })
14+
15+
export class DecorationProvider
16+
extends Disposable
17+
implements FileDecorationProvider
18+
{
19+
private static DecorationFiltered: FileDecoration = {
20+
color: new ThemeColor('gitDecoration.ignoredResourceForeground'),
21+
tooltip: 'Filtered'
22+
}
23+
24+
public readonly onDidChangeFileDecorations: Event<Uri[]>
25+
private readonly decorationsChanged: EventEmitter<Uri[]>
26+
27+
private filtered = new Set<string>()
28+
29+
constructor(decorationsChanged?: EventEmitter<Uri[]>) {
30+
super()
31+
32+
this.decorationsChanged = this.dispose.track(
33+
decorationsChanged || new EventEmitter()
34+
)
35+
this.onDidChangeFileDecorations = this.decorationsChanged.event
36+
37+
this.dispose.track(window.registerFileDecorationProvider(this))
38+
}
39+
40+
public provideFileDecoration(uri: Uri): FileDecoration | undefined {
41+
if (this.filtered.has(uri.fsPath)) {
42+
return DecorationProvider.DecorationFiltered
43+
}
44+
}
45+
46+
public setState(labels: string[], filtered: Set<string>) {
47+
const urisToUpdate: Uri[] = []
48+
49+
for (const label of labels) {
50+
urisToUpdate.push(getDecoratableUri(label))
51+
}
52+
53+
this.filtered = filtered
54+
this.decorationsChanged.fire(urisToUpdate)
55+
}
56+
}

‎extension/src/experiments/model/filterBy/index.test.ts‎

Lines changed: 57 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
import { filterExperiments, Operator } from '.'
1+
import { splitExperimentsByFilters, Operator } from '.'
22
import { buildMetricOrParamPath } from '../../columns/paths'
33
import { Experiment, ColumnType } from '../../webview/contract'
44

5-
describe('filterExperiments', () => {
5+
describe('splitExperimentsByFilters', () => {
66
const paramsFile = 'params.yaml'
77
const experiments = [
88
{
@@ -41,7 +41,10 @@ describe('filterExperiments', () => {
4141
] as unknown as Experiment[]
4242

4343
it('should not filter experiments if they do not have the provided value (for queued experiments)', () => {
44-
const unfilteredQueuedExperiments = filterExperiments(
44+
const {
45+
unfiltered: unfilteredQueuedExperiments,
46+
filtered: filteredQueuedExperiments
47+
} = splitExperimentsByFilters(
4548
[
4649
{
4750
operator: Operator.IS_FALSE,
@@ -67,15 +70,19 @@ describe('filterExperiments', () => {
6770
expect(
6871
unfilteredQueuedExperiments.map(experiment => experiment.id)
6972
).toStrictEqual([1, 2, 3])
73+
expect(
74+
filteredQueuedExperiments.map(experiment => experiment.id)
75+
).toStrictEqual([])
7076
})
7177

72-
it('should return the original experiments if no filters are provided', () => {
73-
const unFilteredExperiments = filterExperiments([], experiments)
74-
expect(unFilteredExperiments).toStrictEqual(experiments)
78+
it('should mark the original experiments as unfiltered if no filters are provided', () => {
79+
const { filtered, unfiltered } = splitExperimentsByFilters([], experiments)
80+
expect(unfiltered).toStrictEqual(experiments)
81+
expect(filtered).toStrictEqual([])
7582
})
7683

77-
it('should filter the experiments by a given filter', () => {
78-
const filteredExperiments = filterExperiments(
84+
it('should split the experiments by a given filter', () => {
85+
const { filtered, unfiltered } = splitExperimentsByFilters(
7986
[
8087
{
8188
operator: Operator.GREATER_THAN,
@@ -85,13 +92,12 @@ describe('filterExperiments', () => {
8592
],
8693
experiments
8794
)
88-
expect(filteredExperiments.map(experiment => experiment.id)).toStrictEqual([
89-
3
90-
])
95+
expect(filtered.map(experiment => experiment.id)).toStrictEqual([1, 2])
96+
expect(unfiltered.map(experiment => experiment.id)).toStrictEqual([3])
9197
})
9298

93-
it('should filter the experiments by an equals filter', () => {
94-
const filteredExperiments = filterExperiments(
99+
it('should split the experiments by an equals filter', () => {
100+
const { filtered, unfiltered } = splitExperimentsByFilters(
95101
[
96102
{
97103
operator: Operator.EQUAL,
@@ -101,13 +107,12 @@ describe('filterExperiments', () => {
101107
],
102108
experiments
103109
)
104-
expect(filteredExperiments.map(experiment => experiment.id)).toStrictEqual([
105-
2
106-
])
110+
expect(filtered.map(experiment => experiment.id)).toStrictEqual([1, 3])
111+
expect(unfiltered.map(experiment => experiment.id)).toStrictEqual([2])
107112
})
108113

109-
it('should filter the experiments by a not equals filter', () => {
110-
const filteredExperiments = filterExperiments(
114+
it('should split the experiments by a not equals filter', () => {
115+
const { filtered, unfiltered } = splitExperimentsByFilters(
111116
[
112117
{
113118
operator: Operator.NOT_EQUAL,
@@ -117,13 +122,12 @@ describe('filterExperiments', () => {
117122
],
118123
experiments
119124
)
120-
expect(filteredExperiments.map(experiment => experiment.id)).toStrictEqual([
121-
1, 3
122-
])
125+
expect(filtered.map(experiment => experiment.id)).toStrictEqual([2])
126+
expect(unfiltered.map(experiment => experiment.id)).toStrictEqual([1, 3])
123127
})
124128

125-
it('should filter the experiments by multiple filters', () => {
126-
const filteredExperiments = filterExperiments(
129+
it('should split the experiments by multiple filters', () => {
130+
const { filtered, unfiltered } = splitExperimentsByFilters(
127131
[
128132
{
129133
operator: Operator.GREATER_THAN,
@@ -138,13 +142,12 @@ describe('filterExperiments', () => {
138142
],
139143
experiments
140144
)
141-
expect(filteredExperiments.map(experiment => experiment.id)).toStrictEqual([
142-
1, 2
143-
])
145+
expect(filtered.map(experiment => experiment.id)).toStrictEqual([3])
146+
expect(unfiltered.map(experiment => experiment.id)).toStrictEqual([1, 2])
144147
})
145148

146-
it('should filter the experiments by multiple filters on multiple params', () => {
147-
const filteredExperiments = filterExperiments(
149+
it('should split the experiments by multiple filters on multiple params', () => {
150+
const { filtered, unfiltered } = splitExperimentsByFilters(
148151
[
149152
{
150153
operator: Operator.GREATER_THAN_OR_EQUAL,
@@ -164,11 +167,12 @@ describe('filterExperiments', () => {
164167
],
165168
experiments
166169
)
167-
expect(filteredExperiments).toStrictEqual([])
170+
expect(filtered).toStrictEqual(experiments)
171+
expect(unfiltered).toStrictEqual([])
168172
})
169173

170-
it('should filter the experiments using string contains', () => {
171-
const experimentsWithText = filterExperiments(
174+
it('should split the experiments using string contains', () => {
175+
const { filtered, unfiltered } = splitExperimentsByFilters(
172176
[
173177
{
174178
operator: Operator.CONTAINS,
@@ -178,13 +182,12 @@ describe('filterExperiments', () => {
178182
],
179183
experiments
180184
)
181-
expect(experimentsWithText.map(experiment => experiment.id)).toStrictEqual([
182-
1
183-
])
185+
expect(filtered.map(experiment => experiment.id)).toStrictEqual([2, 3])
186+
expect(unfiltered.map(experiment => experiment.id)).toStrictEqual([1])
184187
})
185188

186-
it('should filter all experiments if given a numeric column to filter with string contains', () => {
187-
const noExperiments = filterExperiments(
189+
it('should split all experiments if given a numeric column to filter with string contains', () => {
190+
const { filtered, unfiltered } = splitExperimentsByFilters(
188191
[
189192
{
190193
operator: Operator.CONTAINS,
@@ -194,11 +197,12 @@ describe('filterExperiments', () => {
194197
],
195198
experiments
196199
)
197-
expect(noExperiments).toStrictEqual([])
200+
expect(filtered).toStrictEqual(experiments)
201+
expect(unfiltered).toStrictEqual([])
198202
})
199203

200-
it('should not filter any experiments if given a numeric column to filter with string does not contain', () => {
201-
const unfilteredExperiments = filterExperiments(
204+
it('should split the experiments when given a numeric column to filter with string does not contain', () => {
205+
const { filtered, unfiltered } = splitExperimentsByFilters(
202206
[
203207
{
204208
operator: Operator.NOT_CONTAINS,
@@ -208,11 +212,12 @@ describe('filterExperiments', () => {
208212
],
209213
experiments
210214
)
211-
expect(unfilteredExperiments).toStrictEqual(experiments)
215+
expect(filtered).toStrictEqual([])
216+
expect(unfiltered).toStrictEqual(experiments)
212217
})
213218

214-
it('should filter the experiments using string does not contain', () => {
215-
const experimentsWithoutText = filterExperiments(
219+
it('should split the experiments using string does not contain', () => {
220+
const { filtered, unfiltered } = splitExperimentsByFilters(
216221
[
217222
{
218223
operator: Operator.NOT_CONTAINS,
@@ -222,13 +227,12 @@ describe('filterExperiments', () => {
222227
],
223228
experiments
224229
)
225-
expect(
226-
experimentsWithoutText.map(experiment => experiment.id)
227-
).toStrictEqual([2, 3])
230+
expect(filtered.map(experiment => experiment.id)).toStrictEqual([1])
231+
expect(unfiltered.map(experiment => experiment.id)).toStrictEqual([2, 3])
228232
})
229233

230-
it('should filter the experiments using boolean is true', () => {
231-
const experimentsWithTrueBool = filterExperiments(
234+
it('should split the experiments using boolean is true', () => {
235+
const { filtered, unfiltered } = splitExperimentsByFilters(
232236
[
233237
{
234238
operator: Operator.IS_TRUE,
@@ -238,13 +242,12 @@ describe('filterExperiments', () => {
238242
],
239243
experiments
240244
)
241-
expect(
242-
experimentsWithTrueBool.map(experiment => experiment.id)
243-
).toStrictEqual([1])
245+
expect(filtered.map(experiment => experiment.id)).toStrictEqual([2, 3])
246+
expect(unfiltered.map(experiment => experiment.id)).toStrictEqual([1])
244247
})
245248

246-
it('should filter the experiments using boolean is false', () => {
247-
const experimentsWithFalseBool = filterExperiments(
249+
it('should split the experiments using boolean is false', () => {
250+
const { filtered, unfiltered } = splitExperimentsByFilters(
248251
[
249252
{
250253
operator: Operator.IS_FALSE,
@@ -254,8 +257,7 @@ describe('filterExperiments', () => {
254257
],
255258
experiments
256259
)
257-
expect(
258-
experimentsWithFalseBool.map(experiment => experiment.id)
259-
).toStrictEqual([2])
260+
expect(filtered.map(experiment => experiment.id)).toStrictEqual([1, 3])
261+
expect(unfiltered.map(experiment => experiment.id)).toStrictEqual([2])
260262
})
261263
})

0 commit comments

Comments
 (0)