Skip to content

Commit ccd6cab

Browse files
sroy3sroy3
authored
Multiple commits in the experiments table (#2392)
* Show multiple commits * Shorten sha labels from older commits * Fix trees * Fix tests * Fix border * Fix all tests * Add missing change * Fix last border for all situations * Add test
1 parent f5748d1 commit ccd6cab

File tree

16 files changed

+203
-81
lines changed

16 files changed

+203
-81
lines changed

‎extension/src/cli/dvc/constants.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,11 @@ export enum SubCommand {
2828
}
2929

3030
export enum Flag {
31+
ALL_COMMITS = '-A',
3132
FORCE = '-f',
3233
GRANULAR = '--granular',
3334
JSON = '--json',
35+
NUM_COMMIT = '-n',
3436
OUTPUT_PATH = '-o',
3537
SUBDIRECTORY = '--subdir',
3638
SET_PARAM = '-S',

‎extension/src/cli/dvc/reader.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ describe('CliReader', () => {
7272
const cliOutput = await dvcReader.expShow(cwd)
7373
expect(cliOutput).toStrictEqual(expShowFixture)
7474
expect(mockedCreateProcess).toHaveBeenCalledWith({
75-
args: ['exp', 'show', JSON_FLAG],
75+
args: ['exp', 'show', '-n', '3', JSON_FLAG],
7676
cwd,
7777
env: mockedEnv,
7878
executable: 'dvc'

‎extension/src/cli/dvc/reader.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ export interface PlotsOutput {
9292
}
9393

9494
export const TEMP_PLOTS_DIR = join('.dvc', 'tmp', 'plots')
95+
export const NUM_OF_COMMITS_TO_SHOW = '3'
9596

9697
export const isDvcError = <
9798
T extends ExperimentsOutput | DataStatusOutput | PlotsOutput
@@ -135,6 +136,8 @@ export class DvcReader extends DvcCli {
135136
Command.EXPERIMENT,
136137
SubCommand.SHOW,
137138
...flags,
139+
Flag.NUM_COMMIT,
140+
NUM_OF_COMMITS_TO_SHOW,
138141
Flag.JSON
139142
)
140143
if (isDvcError(output) || isEmpty(output)) {

‎extension/src/experiments/index.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,10 @@ export class Experiments extends BaseRepository<TableData> {
363363
return this.experiments.getCheckpointsWithType(id)
364364
}
365365

366+
public getBranchExperiments(branch: Experiment) {
367+
return this.experiments.getExperimentsByBranchForTree(branch)
368+
}
369+
366370
public sendInitialWebviewData() {
367371
return this.webviewMessages.sendWebviewMessage()
368372
}

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -282,8 +282,14 @@ const collectFromBranchesObject = (
282282
for (const [sha, { baseline, ...experimentsObject }] of Object.entries(
283283
branchesObject
284284
)) {
285-
const name = baseline.data?.name || sha
286-
const branch = transformExperimentData(name, baseline, name, sha)
285+
let name = baseline.data?.name
286+
let label = name
287+
288+
if (!name) {
289+
name = sha
290+
label = shortenForLabel(name)
291+
}
292+
const branch = transformExperimentData(name, baseline, label, sha)
287293

288294
if (branch) {
289295
collectFromExperimentsObject(acc, experimentsObject, sha, branch)

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,11 @@ import { definedAndNonEmpty } from '../../../util/array'
88
import { Experiment } from '../../webview/contract'
99

1010
export type ExperimentWithType = Experiment & { type: ExperimentType }
11+
export type ExperimentAugmented = ExperimentWithType & {
12+
hasChildren: boolean
13+
selected?: boolean
14+
starred: boolean
15+
}
1116
export type FilteredCounts = {
1217
checkpoints?: number
1318
experiments: number

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ describe('ExperimentsModel', () => {
145145
}
146146
})
147147

148-
const experiments = model.getExperiments()
148+
const experiments = model.getAllExperiments()
149149

150150
const changed: string[] = []
151151
for (const { deps, sha } of experiments) {

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

Lines changed: 24 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { collectExperiments, collectMutableRevisions } from './collect'
1010
import {
1111
collectFiltered,
1212
collectFilteredCounts,
13+
ExperimentAugmented,
1314
ExperimentWithType
1415
} from './filterBy/collect'
1516
import { collectColoredStatus, collectSelected } from './status/collect'
@@ -228,7 +229,7 @@ export class ExperimentsModel extends ModelWithPersistence {
228229
}
229230

230231
public getMutableRevisions(hasCheckpoints: boolean) {
231-
return collectMutableRevisions(this.getExperiments(), hasCheckpoints)
232+
return collectMutableRevisions(this.getAllExperiments(), hasCheckpoints)
232233
}
233234

234235
public getSelectedRevisions() {
@@ -264,7 +265,7 @@ export class ExperimentsModel extends ModelWithPersistence {
264265

265266
public getUnfilteredExperiments(filters = this.getFilters()) {
266267
const unfilteredExperiments = this.getSubRows(
267-
this.getExperiments(),
268+
this.getAllExperiments(),
268269
filters
269270
)
270271

@@ -285,11 +286,7 @@ export class ExperimentsModel extends ModelWithPersistence {
285286
)
286287
}
287288

288-
public getExperiments(): (ExperimentWithType & {
289-
hasChildren: boolean
290-
selected?: boolean
291-
starred: boolean
292-
})[] {
289+
public getExperiments(): ExperimentAugmented[] {
293290
return [
294291
{
295292
...this.addDetails(this.workspace),
@@ -299,22 +296,17 @@ export class ExperimentsModel extends ModelWithPersistence {
299296
...this.branches.map(branch => {
300297
return {
301298
...this.addDetails(branch),
302-
hasChildren: false,
299+
hasChildren: !!this.experimentsByBranch.get(branch.label),
303300
type: ExperimentType.BRANCH
304301
}
305-
}),
306-
...this.getFlattenedExperiments().map(experiment => ({
307-
...experiment,
308-
hasChildren: definedAndNonEmpty(
309-
this.checkpointsByTip.get(experiment.id)
310-
),
311-
type: experiment.queued
312-
? ExperimentType.QUEUED
313-
: ExperimentType.EXPERIMENT
314-
}))
302+
})
315303
]
316304
}
317305

306+
public getAllExperiments() {
307+
return [...this.getExperiments(), ...this.getFlattenedExperiments()]
308+
}
309+
318310
public getErrors() {
319311
return new Set(
320312
this.getCombinedList()
@@ -325,7 +317,7 @@ export class ExperimentsModel extends ModelWithPersistence {
325317

326318
public getExperimentsWithCheckpoints(): ExperimentWithCheckpoints[] {
327319
const experimentsWithCheckpoints: ExperimentWithCheckpoints[] = []
328-
for (const experiment of this.getExperiments()) {
320+
for (const experiment of this.getAllExperiments()) {
329321
const { id, queued } = experiment
330322
if (queued) {
331323
continue
@@ -342,7 +334,7 @@ export class ExperimentsModel extends ModelWithPersistence {
342334
}
343335

344336
public getExperimentParams(id: string) {
345-
const params = this.getExperiments().find(
337+
const params = this.getAllExperiments().find(
346338
experiment => experiment.id === id
347339
)?.params
348340

@@ -416,6 +408,16 @@ export class ExperimentsModel extends ModelWithPersistence {
416408
]
417409
}
418410

411+
public getExperimentsByBranchForTree(branch: Experiment) {
412+
return this.getExperimentsByBranch(branch)?.map(experiment => ({
413+
...experiment,
414+
hasChildren: definedAndNonEmpty(this.checkpointsByTip.get(experiment.id)),
415+
type: experiment.queued
416+
? ExperimentType.QUEUED
417+
: ExperimentType.EXPERIMENT
418+
}))
419+
}
420+
419421
private getSubRows(experiments: Experiment[], filters = this.getFilters()) {
420422
return experiments
421423
.map(experiment => {
@@ -514,6 +516,7 @@ export class ExperimentsModel extends ModelWithPersistence {
514516
const { coloredStatus, availableColors } = collectColoredStatus(
515517
this.getExperiments(),
516518
this.checkpointsByTip,
519+
this.experimentsByBranch,
517520
this.coloredStatus,
518521
this.availableColors
519522
)
@@ -571,6 +574,7 @@ export class ExperimentsModel extends ModelWithPersistence {
571574
if (!hasKey(this.coloredStatus, id)) {
572575
return {
573576
...experiment,
577+
selected: false,
574578
starred
575579
}
576580
}

‎extension/src/experiments/model/status/collect.test.ts‎

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -13,17 +13,22 @@ describe('collectColoredStatus', () => {
1313
return mockExperiments
1414
}
1515

16-
it('should set new experiments to selected if there are less than 7', () => {
17-
const experiments = buildMockExperiments(4)
18-
const colors = copyOriginalColors()
19-
20-
const { availableColors, coloredStatus } = collectColoredStatus(
16+
const collectedColoredStatus = (experiments: Experiment[]) =>
17+
collectColoredStatus(
2118
experiments,
2219
new Map(),
20+
new Map(),
2321
{},
2422
copyOriginalColors()
2523
)
2624

25+
it('should set new experiments to selected if there are less than 7', () => {
26+
const experiments = buildMockExperiments(4)
27+
const colors = copyOriginalColors()
28+
29+
const { availableColors, coloredStatus } =
30+
collectedColoredStatus(experiments)
31+
2732
expect(availableColors).toStrictEqual(colors.slice(4))
2833
expect(coloredStatus).toStrictEqual({
2934
exp1: colors[0],
@@ -40,12 +45,8 @@ describe('collectColoredStatus', () => {
4045
] as Experiment[]
4146
const colors = copyOriginalColors()
4247

43-
const { availableColors, coloredStatus } = collectColoredStatus(
44-
experiments,
45-
new Map(),
46-
{},
47-
copyOriginalColors()
48-
)
48+
const { availableColors, coloredStatus } =
49+
collectedColoredStatus(experiments)
4950

5051
expect(availableColors).toStrictEqual(colors.slice(1))
5152
expect(coloredStatus).toStrictEqual({
@@ -57,12 +58,8 @@ describe('collectColoredStatus', () => {
5758
const experiments = buildMockExperiments(8)
5859
const colors = copyOriginalColors()
5960

60-
const { availableColors, coloredStatus } = collectColoredStatus(
61-
experiments,
62-
new Map(),
63-
{},
64-
copyOriginalColors()
65-
)
61+
const { availableColors, coloredStatus } =
62+
collectedColoredStatus(experiments)
6663

6764
expect(availableColors).toStrictEqual([])
6865
expect(coloredStatus).toStrictEqual({
@@ -84,6 +81,7 @@ describe('collectColoredStatus', () => {
8481
const { availableColors, coloredStatus } = collectColoredStatus(
8582
experiments,
8683
new Map(),
84+
new Map(),
8785
{
8886
exp2: 0,
8987
exp3: colors[2],
@@ -107,6 +105,7 @@ describe('collectColoredStatus', () => {
107105
const { availableColors, coloredStatus } = collectColoredStatus(
108106
experiments,
109107
new Map(),
108+
new Map(),
110109
{
111110
exp1: 0,
112111
exp10: colors[0],
@@ -138,6 +137,7 @@ describe('collectColoredStatus', () => {
138137
const { availableColors, coloredStatus } = collectColoredStatus(
139138
experiments,
140139
new Map(),
140+
new Map(),
141141
{ exp9: colors[0] },
142142
copyOriginalColors().slice(1)
143143
)
@@ -163,6 +163,7 @@ describe('collectColoredStatus', () => {
163163
const { availableColors, coloredStatus } = collectColoredStatus(
164164
experiments,
165165
new Map(),
166+
new Map(),
166167
{
167168
exp4: colors[0],
168169
exp5: colors[1],
@@ -198,6 +199,7 @@ describe('collectColoredStatus', () => {
198199
const { availableColors, coloredStatus } = collectColoredStatus(
199200
experiments,
200201
checkpointsByTip,
202+
new Map(),
201203
{},
202204
copyOriginalColors()
203205
)
@@ -231,6 +233,7 @@ describe('collectColoredStatus', () => {
231233
const { availableColors, coloredStatus } = collectColoredStatus(
232234
experiments,
233235
checkpointsByTip,
236+
new Map(),
234237
{
235238
checkC1: colors[1],
236239
checkD2: colors[2],

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

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,20 +25,24 @@ const collectStatus = (
2525
if (!id || queued || hasKey(acc, id)) {
2626
return
2727
}
28+
2829
acc[id] = getStatus(acc, unassignColors)
2930
}
3031

3132
const collectExistingStatuses = (
3233
experiments: Experiment[],
3334
checkpointsByTip: Map<string, Experiment[]>,
35+
experimentsByBranch: Map<string, Experiment[]>,
3436
previousStatus: ColoredStatus
3537
) => {
3638
const existingStatuses: ColoredStatus = {}
3739
for (const experiment of [
3840
...experiments,
41+
...flattenMapValues(experimentsByBranch),
3942
...flattenMapValues(checkpointsByTip)
4043
]) {
4144
const { id } = experiment
45+
4246
if (!hasKey(previousStatus, id)) {
4347
continue
4448
}
@@ -70,22 +74,29 @@ export const unassignColors = (
7074
export const collectColoredStatus = (
7175
experiments: Experiment[],
7276
checkpointsByTip: Map<string, Experiment[]>,
77+
experimentsByBranch: Map<string, Experiment[]>,
7378
previousStatus: ColoredStatus,
7479
unassignedColors: Color[]
7580
): { coloredStatus: ColoredStatus; availableColors: Color[] } => {
81+
const flattenExperimentsByBranch = flattenMapValues(experimentsByBranch)
7682
const availableColors = unassignColors(
77-
[...experiments, ...flattenMapValues(checkpointsByTip)],
83+
[
84+
...experiments,
85+
...flattenExperimentsByBranch,
86+
...flattenMapValues(checkpointsByTip)
87+
],
7888
previousStatus,
7989
unassignedColors
8090
)
8191

8292
const coloredStatus = collectExistingStatuses(
8393
experiments,
8494
checkpointsByTip,
95+
experimentsByBranch,
8596
previousStatus
8697
)
8798

88-
for (const experiment of experiments) {
99+
for (const experiment of [...experiments, ...flattenExperimentsByBranch]) {
89100
collectStatus(coloredStatus, experiment, availableColors)
90101

91102
for (const checkpoint of checkpointsByTip.get(experiment.id) || []) {

0 commit comments

Comments
 (0)