Skip to content

Commit f0c5c4e

Browse files
julieg18julieg18
authored
Check hidden status when getting first three exp table column order (#2738)
* have ColumnModel's getFirstThreeColumnOrder check for the columns' hidden status, which stops our experiment tree tooltips and our exp selection quick pick from showing hidden column data.
1 parent 0527b3d commit f0c5c4e

File tree

11 files changed

+163
-63
lines changed

11 files changed

+163
-63
lines changed

‎extension/src/experiments/columns/constants.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,5 @@ export const timestampColumn: Column = {
99
type,
1010
types: [type]
1111
}
12+
13+
export const EXPERIMENT_COLUMN_ID = 'id'

‎extension/src/experiments/columns/model.test.ts‎

Lines changed: 43 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,13 @@ beforeEach(() => {
4242

4343
describe('ColumnsModel', () => {
4444
const exampleDvcRoot = 'test'
45-
const mockedColumnsOrderChanged = buildMockedEventEmitter<void>()
45+
const mockedColumnsOrderOrStatusChanged = buildMockedEventEmitter<void>()
4646

4747
it('should return the expected columns when given the default output fixture', async () => {
4848
const model = new ColumnsModel(
4949
'',
5050
buildMockMemento(),
51-
mockedColumnsOrderChanged
51+
mockedColumnsOrderOrStatusChanged
5252
)
5353
await model.transformAndSet(outputFixture)
5454
expect(mockedGetConfigValue).toHaveBeenCalled()
@@ -59,7 +59,7 @@ describe('ColumnsModel', () => {
5959
const model = new ColumnsModel(
6060
'',
6161
buildMockMemento(),
62-
mockedColumnsOrderChanged
62+
mockedColumnsOrderOrStatusChanged
6363
)
6464
await model.transformAndSet(survivalOutputFixture)
6565
expect(model.getSelected()).toStrictEqual(survivalColumnsFixture)
@@ -69,7 +69,7 @@ describe('ColumnsModel', () => {
6969
const model = new ColumnsModel(
7070
'',
7171
buildMockMemento(),
72-
mockedColumnsOrderChanged
72+
mockedColumnsOrderOrStatusChanged
7373
)
7474
await model.transformAndSet(deeplyNestedOutputFixture)
7575
expect(mockedGetConfigValue).toHaveBeenCalled()
@@ -81,7 +81,7 @@ describe('ColumnsModel', () => {
8181
const model = new ColumnsModel(
8282
'',
8383
buildMockMemento(),
84-
mockedColumnsOrderChanged
84+
mockedColumnsOrderOrStatusChanged
8585
)
8686
await model.transformAndSet(deeplyNestedOutputFixture)
8787
expect(mockedGetConfigValue).toHaveBeenCalled()
@@ -93,7 +93,7 @@ describe('ColumnsModel', () => {
9393
const model = new ColumnsModel(
9494
'',
9595
buildMockMemento(),
96-
mockedColumnsOrderChanged
96+
mockedColumnsOrderOrStatusChanged
9797
)
9898
await model.transformAndSet(deeplyNestedOutputFixture)
9999
expect(mockedGetConfigValue).toHaveBeenCalled()
@@ -105,7 +105,7 @@ describe('ColumnsModel', () => {
105105
const model = new ColumnsModel(
106106
'',
107107
buildMockMemento(),
108-
mockedColumnsOrderChanged
108+
mockedColumnsOrderOrStatusChanged
109109
)
110110
await model.transformAndSet(deeplyNestedOutputFixture)
111111
expect(mockedGetConfigValue).toHaveBeenCalled()
@@ -117,7 +117,7 @@ describe('ColumnsModel', () => {
117117
const model = new ColumnsModel(
118118
'',
119119
buildMockMemento(),
120-
mockedColumnsOrderChanged
120+
mockedColumnsOrderOrStatusChanged
121121
)
122122
await model.transformAndSet(deeplyNestedOutputFixture)
123123
expect(mockedGetConfigValue).toHaveBeenCalled()
@@ -129,7 +129,7 @@ describe('ColumnsModel', () => {
129129
const model = new ColumnsModel(
130130
'',
131131
buildMockMemento(),
132-
mockedColumnsOrderChanged
132+
mockedColumnsOrderOrStatusChanged
133133
)
134134
await model.transformAndSet(deeplyNestedOutputFixture)
135135
expect(mockedGetConfigValue).toHaveBeenCalled()
@@ -141,7 +141,7 @@ describe('ColumnsModel', () => {
141141
const model = new ColumnsModel(
142142
'',
143143
buildMockMemento(),
144-
mockedColumnsOrderChanged
144+
mockedColumnsOrderOrStatusChanged
145145
)
146146
await model.transformAndSet(deeplyNestedOutputFixture)
147147
expect(mockedGetConfigValue).toHaveBeenCalled()
@@ -152,7 +152,7 @@ describe('ColumnsModel', () => {
152152
const model = new ColumnsModel(
153153
'',
154154
buildMockMemento(),
155-
mockedColumnsOrderChanged
155+
mockedColumnsOrderOrStatusChanged
156156
)
157157
await model.transformAndSet(dataTypesOutputFixture)
158158
expect(model.getSelected()).toStrictEqual(dataTypesColumnsFixture)
@@ -183,7 +183,7 @@ describe('ColumnsModel', () => {
183183
const model = new ColumnsModel(
184184
exampleDvcRoot,
185185
buildMockMemento(),
186-
mockedColumnsOrderChanged
186+
mockedColumnsOrderOrStatusChanged
187187
)
188188
await model.transformAndSet(exampleData)
189189
expect(model.getSelected()).toStrictEqual([
@@ -217,7 +217,7 @@ describe('ColumnsModel', () => {
217217
[testParamPath]: Status.UNSELECTED
218218
}
219219
}),
220-
mockedColumnsOrderChanged
220+
mockedColumnsOrderOrStatusChanged
221221
)
222222
await model.transformAndSet(exampleData)
223223
expect(model.getSelected()).toStrictEqual([
@@ -246,19 +246,20 @@ describe('ColumnsModel', () => {
246246
[PersistenceKey.METRICS_AND_PARAMS_COLUMN_ORDER + exampleDvcRoot]:
247247
persistedState
248248
}),
249-
mockedColumnsOrderChanged
249+
mockedColumnsOrderOrStatusChanged
250250
)
251251
expect(model.getColumnOrder()).toStrictEqual(persistedState)
252252
})
253253

254-
it('should return the first three columns from the persisted state', () => {
254+
it('should return the first three none hidden columns from the persisted state', async () => {
255255
const persistedState = [
256-
{ path: 'A', width: 0 },
257-
{ path: 'B', width: 0 },
258-
{ path: 'C', width: 0 },
259-
{ path: 'D', width: 0 },
260-
{ path: 'E', width: 0 },
261-
{ path: 'F', width: 0 }
256+
'id',
257+
'Created',
258+
'params:params.yaml:dvc_logs_dir',
259+
'params:params.yaml:process.threshold',
260+
'params:params.yaml:process.test_arg',
261+
'deps:src/prepare.py',
262+
'deps:src/featurization.py'
262263
]
263264

264265
const model = new ColumnsModel(
@@ -267,19 +268,26 @@ describe('ColumnsModel', () => {
267268
[PersistenceKey.METRICS_AND_PARAMS_COLUMN_ORDER + exampleDvcRoot]:
268269
persistedState
269270
}),
270-
mockedColumnsOrderChanged
271+
mockedColumnsOrderOrStatusChanged
271272
)
273+
await model.transformAndSet(outputFixture)
272274

273275
expect(model.getFirstThreeColumnOrder()).toStrictEqual(
274276
persistedState.slice(1, 4)
275277
)
278+
279+
model.toggleStatus('Created')
280+
281+
expect(model.getFirstThreeColumnOrder()).toStrictEqual(
282+
persistedState.slice(2, 5)
283+
)
276284
})
277285

278-
it('should return first three columns collected from data if state is empty', async () => {
286+
it('should return the first three none hidden columns collected from data if state is empty', async () => {
279287
const model = new ColumnsModel(
280288
exampleDvcRoot,
281289
buildMockMemento(),
282-
mockedColumnsOrderChanged
290+
mockedColumnsOrderOrStatusChanged
283291
)
284292
await model.transformAndSet(outputFixture)
285293

@@ -288,6 +296,14 @@ describe('ColumnsModel', () => {
288296
'metrics:summary.json:loss',
289297
'metrics:summary.json:accuracy'
290298
])
299+
300+
model.toggleStatus('Created')
301+
302+
expect(model.getFirstThreeColumnOrder()).toStrictEqual([
303+
'metrics:summary.json:loss',
304+
'metrics:summary.json:accuracy',
305+
'metrics:summary.json:val_loss'
306+
])
291307
})
292308

293309
it('should re-order the columns if a new columnOrder is set', () => {
@@ -300,7 +316,7 @@ describe('ColumnsModel', () => {
300316
{ path: 'C', width: 0 }
301317
]
302318
}),
303-
mockedColumnsOrderChanged
319+
mockedColumnsOrderOrStatusChanged
304320
)
305321
const newState = ['C', 'B', 'A']
306322
model.setColumnOrder(newState)
@@ -321,7 +337,7 @@ describe('ColumnsModel', () => {
321337
[PersistenceKey.METRICS_AND_PARAMS_COLUMN_ORDER + exampleDvcRoot]:
322338
persistedState
323339
}),
324-
mockedColumnsOrderChanged
340+
mockedColumnsOrderOrStatusChanged
325341
)
326342
expect(model.getColumnOrder()).toStrictEqual(persistedState)
327343
})
@@ -338,7 +354,7 @@ describe('ColumnsModel', () => {
338354
[PersistenceKey.METRICS_AND_PARAMS_COLUMN_ORDER + exampleDvcRoot]:
339355
persistedState
340356
}),
341-
mockedColumnsOrderChanged
357+
mockedColumnsOrderOrStatusChanged
342358
)
343359
const changedColumnId = 'C'
344360
const expectedWidth = 77

‎extension/src/experiments/columns/model.ts‎

Lines changed: 51 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
import { EventEmitter, Memento } from 'vscode'
22
import { collectChanges, collectColumns, collectParamsFiles } from './collect'
3+
import { EXPERIMENT_COLUMN_ID, timestampColumn } from './constants'
34
import { Column, ColumnType } from '../webview/contract'
45
import { ExperimentsOutput } from '../../cli/dvc/contract'
56
import { PersistenceKey } from '../../persistence/constants'
67
import { PathSelectionModel } from '../../path/selection/model'
78

89
export class ColumnsModel extends PathSelectionModel<Column> {
9-
private columnsOrderChanged: EventEmitter<void>
1010
private columnOrderState: string[] = []
1111
private columnWidthsState: Record<string, number> = {}
1212
private columnsChanges: string[] = []
@@ -15,9 +15,14 @@ export class ColumnsModel extends PathSelectionModel<Column> {
1515
constructor(
1616
dvcRoot: string,
1717
workspaceState: Memento,
18-
columnsOrderChanged: EventEmitter<void>
18+
columnsOrderOrStatusChanged: EventEmitter<void>
1919
) {
20-
super(dvcRoot, workspaceState, PersistenceKey.METRICS_AND_PARAMS_STATUS)
20+
super(
21+
dvcRoot,
22+
workspaceState,
23+
PersistenceKey.METRICS_AND_PARAMS_STATUS,
24+
columnsOrderOrStatusChanged
25+
)
2126

2227
this.columnOrderState = this.revive(
2328
PersistenceKey.METRICS_AND_PARAMS_COLUMN_ORDER,
@@ -27,17 +32,16 @@ export class ColumnsModel extends PathSelectionModel<Column> {
2732
PersistenceKey.METRICS_AND_PARAMS_COLUMN_WIDTHS,
2833
{}
2934
)
30-
this.columnsOrderChanged = columnsOrderChanged
3135
}
3236

3337
public getColumnOrder(): string[] {
3438
return this.columnOrderState
3539
}
3640

3741
public getFirstThreeColumnOrder(): string[] {
38-
return this.columnOrderState.length === 0
39-
? this.getFirstThreeColumnOrderFromData()
40-
: this.columnOrderState.slice(1, 4)
42+
return this.columnOrderState
43+
.filter(path => this.status[path] && this.status[path] === 2)
44+
.slice(0, 3)
4145
}
4246

4347
public getColumnWidths(): Record<string, number> {
@@ -65,7 +69,7 @@ export class ColumnsModel extends PathSelectionModel<Column> {
6569
PersistenceKey.METRICS_AND_PARAMS_COLUMN_ORDER,
6670
this.getColumnOrder()
6771
)
68-
this.columnsOrderChanged.fire()
72+
this.statusChanged?.fire()
6973
}
7074

7175
public setColumnWidth(id: string, width: number) {
@@ -91,13 +95,6 @@ export class ColumnsModel extends PathSelectionModel<Column> {
9195
return this.data.length > 1
9296
}
9397

94-
private getFirstThreeColumnOrderFromData(): string[] {
95-
return this.data
96-
.filter(({ hasChildren }) => !hasChildren)
97-
.slice(0, 3)
98-
.map(({ path }) => path)
99-
}
100-
10198
private filterChildren(path?: string) {
10299
return this.data.filter(element =>
103100
path
@@ -108,6 +105,40 @@ export class ColumnsModel extends PathSelectionModel<Column> {
108105
)
109106
}
110107

108+
private findChildrenColumns(
109+
parent: string,
110+
columns: Column[],
111+
childrenColumns: string[]
112+
) {
113+
const filteredColumns = columns.filter(
114+
({ parentPath }) => parentPath === parent
115+
)
116+
for (const column of filteredColumns) {
117+
if (column.hasChildren) {
118+
this.findChildrenColumns(column.path, columns, childrenColumns)
119+
} else {
120+
childrenColumns.push(column.path)
121+
}
122+
}
123+
}
124+
125+
private getColumnsFromType(type: string): string[] {
126+
const childrenColumns: string[] = []
127+
const dataWithType = this.data.filter(({ path }) => path.startsWith(type))
128+
this.findChildrenColumns(type, dataWithType, childrenColumns)
129+
return childrenColumns
130+
}
131+
132+
private getColumnOrderFromData() {
133+
return [
134+
EXPERIMENT_COLUMN_ID,
135+
timestampColumn.path,
136+
...this.getColumnsFromType(ColumnType.METRICS),
137+
...this.getColumnsFromType(ColumnType.PARAMS),
138+
...this.getColumnsFromType(ColumnType.DEPS)
139+
]
140+
}
141+
111142
private async transformAndSetColumns(data: ExperimentsOutput) {
112143
const [columns, paramsFiles] = await Promise.all([
113144
collectColumns(data),
@@ -117,6 +148,11 @@ export class ColumnsModel extends PathSelectionModel<Column> {
117148
this.setNewStatuses(columns)
118149

119150
this.data = columns
151+
152+
if (this.columnOrderState.length === 0) {
153+
this.setColumnOrder(this.getColumnOrderFromData())
154+
}
155+
120156
this.paramsFiles = paramsFiles
121157
}
122158

‎extension/src/experiments/index.ts‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ export class Experiments extends BaseRepository<TableData> {
5454
public readonly onDidChangeIsParamsFileFocused: Event<string | undefined>
5555
public readonly onDidChangeExperiments: Event<ExperimentsOutput | void>
5656
public readonly onDidChangeColumns: Event<void>
57-
public readonly onDidChangeColumnOrder: Event<void>
57+
public readonly onDidChangeColumnOrderOrStatus: Event<void>
5858
public readonly onDidChangeCheckpoints: Event<void>
5959

6060
public readonly viewKey = ViewKey.EXPERIMENTS
@@ -79,7 +79,7 @@ export class Experiments extends BaseRepository<TableData> {
7979
)
8080

8181
private readonly columnsChanged = this.dispose.track(new EventEmitter<void>())
82-
private readonly columnsOrderChanged = this.dispose.track(
82+
private readonly columnsOrderOrStatusChanged = this.dispose.track(
8383
new EventEmitter<void>()
8484
)
8585

@@ -106,15 +106,19 @@ export class Experiments extends BaseRepository<TableData> {
106106
this.onDidChangeIsParamsFileFocused = this.paramsFileFocused.event
107107
this.onDidChangeExperiments = this.experimentsChanged.event
108108
this.onDidChangeColumns = this.columnsChanged.event
109-
this.onDidChangeColumnOrder = this.columnsOrderChanged.event
109+
this.onDidChangeColumnOrderOrStatus = this.columnsOrderOrStatusChanged.event
110110
this.onDidChangeCheckpoints = this.checkpointsChanged.event
111111

112112
this.experiments = this.dispose.track(
113113
new ExperimentsModel(dvcRoot, workspaceState)
114114
)
115115

116116
this.columns = this.dispose.track(
117-
new ColumnsModel(dvcRoot, workspaceState, this.columnsOrderChanged)
117+
new ColumnsModel(
118+
dvcRoot,
119+
workspaceState,
120+
this.columnsOrderOrStatusChanged
121+
)
118122
)
119123

120124
this.checkpoints = this.dispose.track(new CheckpointsModel())

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,8 @@ export class ExperimentsTree
358358
return value !== null ? `| ${truncatedKey} | ${value} |\n` : ''
359359
})
360360
.join('')
361-
return getMarkdownString(`|||\n|:--|--|\n${data}`)
361+
362+
return data === '' ? undefined : getMarkdownString(`|||\n|:--|--|\n${data}`)
362363
}
363364

364365
private getTooltip(

0 commit comments

Comments
 (0)