Skip to content

Commit a938b55

Browse files
authored
Add filtered counts to Filter By tree (#1866)
* add filtered counts to the filter by tree description * refactor text collection * add integration test
1 parent 4684af2 commit a938b55

File tree

5 files changed

+209
-12
lines changed

5 files changed

+209
-12
lines changed

extension/src/experiments/index.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,10 @@ export class Experiments extends BaseRepository<TableData> {
257257
return this.notifyChanged()
258258
}
259259

260+
public getFilteredCounts() {
261+
return this.experiments.getFilteredCounts()
262+
}
263+
260264
public getExperimentCount() {
261265
return this.experiments.getExperimentCount()
262266
}

extension/src/experiments/model/filterBy/tree.ts

Lines changed: 62 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,19 +3,20 @@ import {
33
ThemeIcon,
44
TreeDataProvider,
55
TreeItem,
6-
TreeItemCollapsibleState
6+
TreeItemCollapsibleState,
7+
TreeView
78
} from 'vscode'
89
import { getFilterId } from '.'
910
import { WorkspaceExperiments } from '../../workspace'
1011
import { RegisteredCommands } from '../../../commands/external'
1112
import { InternalCommands } from '../../../commands/internal'
1213
import { sendViewOpenedTelemetryEvent } from '../../../telemetry'
1314
import { EventName } from '../../../telemetry/constants'
14-
import { definedAndNonEmpty } from '../../../util/array'
15+
import { definedAndNonEmpty, joinTruthyItems } from '../../../util/array'
1516
import { createTreeView, getRootItem } from '../../../vscode/tree'
1617
import { Disposable } from '../../../class/dispose'
1718

18-
type FilterItem = {
19+
export type FilterItem = {
1920
description: string
2021
dvcRoot: string
2122
id: string
@@ -29,6 +30,7 @@ export class ExperimentsFilterByTree
2930
public readonly onDidChangeTreeData: Event<string | void>
3031

3132
private readonly experiments: WorkspaceExperiments
33+
private readonly view: TreeView<string | FilterItem>
3234
private viewed = false
3335

3436
constructor(
@@ -39,7 +41,7 @@ export class ExperimentsFilterByTree
3941

4042
this.onDidChangeTreeData = experiments.experimentsChanged.event
4143

42-
this.dispose.track(
44+
this.view = this.dispose.track(
4345
createTreeView<FilterItem>('dvc.views.experimentsFilterByTree', this)
4446
)
4547

@@ -54,6 +56,8 @@ export class ExperimentsFilterByTree
5456
RegisteredCommands.EXPERIMENT_FILTERS_REMOVE_ALL,
5557
resource => this.removeAllFilters(resource)
5658
)
59+
60+
this.updateDescriptionOnChange()
5761
}
5862

5963
public getTreeItem(element: string | FilterItem): TreeItem {
@@ -139,4 +143,58 @@ export class ExperimentsFilterByTree
139143
private getDvcRoots() {
140144
return this.experiments.getDvcRoots()
141145
}
146+
147+
private updateDescriptionOnChange() {
148+
this.dispose.track(
149+
this.onDidChangeTreeData(() => {
150+
this.view.description = this.getDescription()
151+
})
152+
)
153+
}
154+
155+
private getDescription() {
156+
const dvcRoots = this.experiments.getDvcRoots()
157+
if (!definedAndNonEmpty(dvcRoots)) {
158+
return
159+
}
160+
161+
const filtered = { checkpoints: 0, experiments: 0 }
162+
163+
for (const dvcRoot of dvcRoots) {
164+
const { experiments, checkpoints } = this.experiments
165+
.getRepository(dvcRoot)
166+
.getFilteredCounts()
167+
filtered.checkpoints = filtered.checkpoints + checkpoints
168+
filtered.experiments = filtered.experiments + experiments
169+
}
170+
171+
const combinedText = joinTruthyItems(
172+
[
173+
this.getDescriptionText('Experiment', filtered.experiments),
174+
this.getDescriptionText('Checkpoint', filtered.checkpoints)
175+
],
176+
', '
177+
)
178+
179+
if (!combinedText) {
180+
return
181+
}
182+
183+
return `${combinedText} Filtered`
184+
}
185+
186+
private getDescriptionText(
187+
type: 'Experiment' | 'Checkpoint',
188+
filteredCount: number
189+
) {
190+
if (!filteredCount) {
191+
return
192+
}
193+
194+
const text = `${filteredCount} ${type}`
195+
if (filteredCount === 1) {
196+
return text
197+
}
198+
return text + 's'
199+
}
142200
}

extension/src/experiments/model/index.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -348,6 +348,29 @@ export class ExperimentsModel extends ModelWithPersistence {
348348
])
349349
}
350350

351+
public getFilteredCounts() {
352+
const experiments = this.flattenExperiments()
353+
const totalExperiments = experiments.length
354+
355+
const remainingExperiments = filterExperiments(
356+
this.getFilters(),
357+
experiments
358+
).length
359+
360+
const checkpoints = this.flattenCheckpoints()
361+
const totalCheckpoints = checkpoints.length
362+
363+
const remainingCheckpoints = filterExperiments(
364+
this.getFilters(),
365+
checkpoints
366+
).length
367+
368+
return {
369+
checkpoints: totalCheckpoints - remainingCheckpoints,
370+
experiments: totalExperiments - remainingExperiments
371+
}
372+
}
373+
351374
private getCombinedList() {
352375
return [
353376
this.workspace,

extension/src/test/suite/experiments/model/filterBy/tree.test.ts

Lines changed: 107 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
import { afterEach, beforeEach, describe, it, suite } from 'mocha'
22
import { expect } from 'chai'
33
import { stub, spy, restore } from 'sinon'
4-
import { window, commands, MessageItem } from 'vscode'
5-
import { addFilterViaQuickInput } from './util'
4+
import { window, commands, MessageItem, EventEmitter, TreeView } from 'vscode'
5+
import { addFilterViaQuickInput, mockQuickInputFilter } from './util'
66
import { Disposable } from '../../../../../extension'
77
import columnsFixture from '../../../../fixtures/expShow/columns'
88
import rowsFixture from '../../../../fixtures/expShow/rows'
@@ -12,7 +12,7 @@ import {
1212
getFilterId,
1313
Operator
1414
} from '../../../../../experiments/model/filterBy'
15-
import { dvcDemoPath } from '../../../../util'
15+
import { buildMockMemento, dvcDemoPath } from '../../../../util'
1616
import { experimentsUpdatedEvent } from '../../../util'
1717
import { buildMetricOrParamPath } from '../../../../../experiments/columns/paths'
1818
import { RegisteredCommands } from '../../../../../commands/external'
@@ -25,6 +25,10 @@ import { WEBVIEW_TEST_TIMEOUT } from '../../../timeouts'
2525
import { Response } from '../../../../../vscode/response'
2626
import { ExperimentsModel } from '../../../../../experiments/model'
2727
import { Title } from '../../../../../vscode/title'
28+
import {
29+
ExperimentsFilterByTree,
30+
FilterItem
31+
} from '../../../../../experiments/model/filterBy/tree'
2832

2933
suite('Experiments Filter By Tree Test Suite', () => {
3034
const disposable = Disposable.fn()
@@ -345,5 +349,105 @@ suite('Experiments Filter By Tree Test Suite', () => {
345349
'should not call get repository in removeFilters without a root'
346350
).not.to.be.called
347351
})
352+
353+
it('should update the description when a filter is added or removed', async () => {
354+
const { experiments, internalCommands } = buildExperiments(disposable)
355+
await experiments.isReady()
356+
357+
const workspaceExperiments = disposable.track(
358+
new WorkspaceExperiments(
359+
internalCommands,
360+
disposable.track(new EventEmitter()),
361+
buildMockMemento(),
362+
{ [dvcDemoPath]: experiments },
363+
disposable.track(new EventEmitter())
364+
)
365+
)
366+
disposable.track(
367+
experiments.onDidChangeExperiments(() =>
368+
workspaceExperiments.experimentsChanged.fire()
369+
)
370+
)
371+
372+
const mockTreeView = {
373+
description: undefined,
374+
dispose: stub()
375+
} as unknown as TreeView<string | FilterItem>
376+
stub(window, 'createTreeView').returns(mockTreeView)
377+
378+
stub(internalCommands, 'registerExternalCommand').returns(undefined)
379+
disposable.track(
380+
new ExperimentsFilterByTree(workspaceExperiments, internalCommands)
381+
)
382+
const getUpdateEvent = () =>
383+
new Promise(resolve =>
384+
disposable.track(
385+
workspaceExperiments.onDidChangeExperiments(() =>
386+
resolve(undefined)
387+
)
388+
)
389+
)
390+
391+
const tableFilterAdded = getUpdateEvent()
392+
393+
const filter = {
394+
operator: Operator.EQUAL,
395+
path: buildMetricOrParamPath(
396+
ColumnType.METRICS,
397+
'summary.json',
398+
'loss'
399+
),
400+
value: '0'
401+
}
402+
403+
mockQuickInputFilter(filter)
404+
experiments.addFilter()
405+
await tableFilterAdded
406+
407+
expect(experiments.getFilteredCounts()).to.deep.equal({
408+
checkpoints: 9,
409+
experiments: 3
410+
})
411+
412+
expect(mockTreeView.description).to.equal(
413+
'3 Experiments, 9 Checkpoints Filtered'
414+
)
415+
416+
const tableFilterRemoved = getUpdateEvent()
417+
experiments.removeFilter(getFilterId(filter))
418+
419+
await tableFilterRemoved
420+
421+
expect(experiments.getFilteredCounts()).to.deep.equal({
422+
checkpoints: 0,
423+
experiments: 0
424+
})
425+
426+
expect(mockTreeView.description).to.be.undefined
427+
428+
stub(experiments, 'getFilteredCounts')
429+
.onFirstCall()
430+
.returns({
431+
checkpoints: 1,
432+
experiments: 0
433+
})
434+
.onSecondCall()
435+
.returns({
436+
checkpoints: 0,
437+
experiments: 1
438+
})
439+
440+
const allButOneCheckpointFilteredEvent = getUpdateEvent()
441+
workspaceExperiments.experimentsChanged.fire()
442+
await allButOneCheckpointFilteredEvent
443+
444+
expect(mockTreeView.description).to.equal('1 Checkpoint Filtered')
445+
446+
const allButOneExperimentFilteredEvent = getUpdateEvent()
447+
workspaceExperiments.experimentsChanged.fire()
448+
await allButOneExperimentFilteredEvent
449+
450+
expect(mockTreeView.description).to.equal('1 Experiment Filtered')
451+
})
348452
})
349453
})

extension/src/test/suite/experiments/model/filterBy/util.ts

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,11 @@ import { experimentsUpdatedEvent } from '../../../util'
66
import { Experiments } from '../../../../../experiments'
77
import { RegisteredCommands } from '../../../../../commands/external'
88

9-
export const addFilterViaQuickInput = (
10-
experiments: Experiments,
9+
export const mockQuickInputFilter = (
1110
fixtureFilter: FilterDefinition,
1211
mockShowQuickPick = stub(window, 'showQuickPick'),
1312
mockShowInputBox = stub(window, 'showInputBox')
1413
) => {
15-
mockShowQuickPick.resetHistory()
16-
mockShowInputBox.resetHistory()
17-
1814
const column = columnsFixture.find(
1915
column => column.path === fixtureFilter.path
2016
)
@@ -25,6 +21,18 @@ export const addFilterViaQuickInput = (
2521
value: fixtureFilter.operator
2622
} as unknown as QuickPickItem)
2723
mockShowInputBox.onFirstCall().resolves(fixtureFilter.value as string)
24+
}
25+
26+
export const addFilterViaQuickInput = (
27+
experiments: Experiments,
28+
fixtureFilter: FilterDefinition,
29+
mockShowQuickPick = stub(window, 'showQuickPick'),
30+
mockShowInputBox = stub(window, 'showInputBox')
31+
) => {
32+
mockShowQuickPick.resetHistory()
33+
mockShowInputBox.resetHistory()
34+
35+
mockQuickInputFilter(fixtureFilter, mockShowQuickPick, mockShowInputBox)
2836

2937
const tableFilterAdded = experimentsUpdatedEvent(experiments)
3038

0 commit comments

Comments
 (0)