Skip to content

Commit

Permalink
Multiple commits in the experiments table (#2392)
Browse files Browse the repository at this point in the history
* 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
sroy3 authored Sep 23, 2022

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
1 parent f5748d1 commit ccd6cab
Showing 16 changed files with 203 additions and 81 deletions.
2 changes: 2 additions & 0 deletions extension/src/cli/dvc/constants.ts
Original file line number Diff line number Diff line change
@@ -28,9 +28,11 @@ export enum SubCommand {
}

export enum Flag {
ALL_COMMITS = '-A',
FORCE = '-f',
GRANULAR = '--granular',
JSON = '--json',
NUM_COMMIT = '-n',
OUTPUT_PATH = '-o',
SUBDIRECTORY = '--subdir',
SET_PARAM = '-S',
2 changes: 1 addition & 1 deletion extension/src/cli/dvc/reader.test.ts
Original file line number Diff line number Diff line change
@@ -72,7 +72,7 @@ describe('CliReader', () => {
const cliOutput = await dvcReader.expShow(cwd)
expect(cliOutput).toStrictEqual(expShowFixture)
expect(mockedCreateProcess).toHaveBeenCalledWith({
args: ['exp', 'show', JSON_FLAG],
args: ['exp', 'show', '-n', '3', JSON_FLAG],
cwd,
env: mockedEnv,
executable: 'dvc'
3 changes: 3 additions & 0 deletions extension/src/cli/dvc/reader.ts
Original file line number Diff line number Diff line change
@@ -92,6 +92,7 @@ export interface PlotsOutput {
}

export const TEMP_PLOTS_DIR = join('.dvc', 'tmp', 'plots')
export const NUM_OF_COMMITS_TO_SHOW = '3'

export const isDvcError = <
T extends ExperimentsOutput | DataStatusOutput | PlotsOutput
@@ -135,6 +136,8 @@ export class DvcReader extends DvcCli {
Command.EXPERIMENT,
SubCommand.SHOW,
...flags,
Flag.NUM_COMMIT,
NUM_OF_COMMITS_TO_SHOW,
Flag.JSON
)
if (isDvcError(output) || isEmpty(output)) {
4 changes: 4 additions & 0 deletions extension/src/experiments/index.ts
Original file line number Diff line number Diff line change
@@ -363,6 +363,10 @@ export class Experiments extends BaseRepository<TableData> {
return this.experiments.getCheckpointsWithType(id)
}

public getBranchExperiments(branch: Experiment) {
return this.experiments.getExperimentsByBranchForTree(branch)
}

public sendInitialWebviewData() {
return this.webviewMessages.sendWebviewMessage()
}
10 changes: 8 additions & 2 deletions extension/src/experiments/model/collect.ts
Original file line number Diff line number Diff line change
@@ -282,8 +282,14 @@ const collectFromBranchesObject = (
for (const [sha, { baseline, ...experimentsObject }] of Object.entries(
branchesObject
)) {
const name = baseline.data?.name || sha
const branch = transformExperimentData(name, baseline, name, sha)
let name = baseline.data?.name
let label = name

if (!name) {
name = sha
label = shortenForLabel(name)
}
const branch = transformExperimentData(name, baseline, label, sha)

if (branch) {
collectFromExperimentsObject(acc, experimentsObject, sha, branch)
5 changes: 5 additions & 0 deletions extension/src/experiments/model/filterBy/collect.ts
Original file line number Diff line number Diff line change
@@ -8,6 +8,11 @@ import { definedAndNonEmpty } from '../../../util/array'
import { Experiment } from '../../webview/contract'

export type ExperimentWithType = Experiment & { type: ExperimentType }
export type ExperimentAugmented = ExperimentWithType & {
hasChildren: boolean
selected?: boolean
starred: boolean
}
export type FilteredCounts = {
checkpoints?: number
experiments: number
2 changes: 1 addition & 1 deletion extension/src/experiments/model/index.test.ts
Original file line number Diff line number Diff line change
@@ -145,7 +145,7 @@ describe('ExperimentsModel', () => {
}
})

const experiments = model.getExperiments()
const experiments = model.getAllExperiments()

const changed: string[] = []
for (const { deps, sha } of experiments) {
44 changes: 24 additions & 20 deletions extension/src/experiments/model/index.ts
Original file line number Diff line number Diff line change
@@ -10,6 +10,7 @@ import { collectExperiments, collectMutableRevisions } from './collect'
import {
collectFiltered,
collectFilteredCounts,
ExperimentAugmented,
ExperimentWithType
} from './filterBy/collect'
import { collectColoredStatus, collectSelected } from './status/collect'
@@ -228,7 +229,7 @@ export class ExperimentsModel extends ModelWithPersistence {
}

public getMutableRevisions(hasCheckpoints: boolean) {
return collectMutableRevisions(this.getExperiments(), hasCheckpoints)
return collectMutableRevisions(this.getAllExperiments(), hasCheckpoints)
}

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

public getUnfilteredExperiments(filters = this.getFilters()) {
const unfilteredExperiments = this.getSubRows(
this.getExperiments(),
this.getAllExperiments(),
filters
)

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

public getExperiments(): (ExperimentWithType & {
hasChildren: boolean
selected?: boolean
starred: boolean
})[] {
public getExperiments(): ExperimentAugmented[] {
return [
{
...this.addDetails(this.workspace),
@@ -299,22 +296,17 @@ export class ExperimentsModel extends ModelWithPersistence {
...this.branches.map(branch => {
return {
...this.addDetails(branch),
hasChildren: false,
hasChildren: !!this.experimentsByBranch.get(branch.label),
type: ExperimentType.BRANCH
}
}),
...this.getFlattenedExperiments().map(experiment => ({
...experiment,
hasChildren: definedAndNonEmpty(
this.checkpointsByTip.get(experiment.id)
),
type: experiment.queued
? ExperimentType.QUEUED
: ExperimentType.EXPERIMENT
}))
})
]
}

public getAllExperiments() {
return [...this.getExperiments(), ...this.getFlattenedExperiments()]
}

public getErrors() {
return new Set(
this.getCombinedList()
@@ -325,7 +317,7 @@ export class ExperimentsModel extends ModelWithPersistence {

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

public getExperimentParams(id: string) {
const params = this.getExperiments().find(
const params = this.getAllExperiments().find(
experiment => experiment.id === id
)?.params

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

public getExperimentsByBranchForTree(branch: Experiment) {
return this.getExperimentsByBranch(branch)?.map(experiment => ({
...experiment,
hasChildren: definedAndNonEmpty(this.checkpointsByTip.get(experiment.id)),
type: experiment.queued
? ExperimentType.QUEUED
: ExperimentType.EXPERIMENT
}))
}

private getSubRows(experiments: Experiment[], filters = this.getFilters()) {
return experiments
.map(experiment => {
@@ -514,6 +516,7 @@ export class ExperimentsModel extends ModelWithPersistence {
const { coloredStatus, availableColors } = collectColoredStatus(
this.getExperiments(),
this.checkpointsByTip,
this.experimentsByBranch,
this.coloredStatus,
this.availableColors
)
@@ -571,6 +574,7 @@ export class ExperimentsModel extends ModelWithPersistence {
if (!hasKey(this.coloredStatus, id)) {
return {
...experiment,
selected: false,
starred
}
}
37 changes: 20 additions & 17 deletions extension/src/experiments/model/status/collect.test.ts
Original file line number Diff line number Diff line change
@@ -13,17 +13,22 @@ describe('collectColoredStatus', () => {
return mockExperiments
}

it('should set new experiments to selected if there are less than 7', () => {
const experiments = buildMockExperiments(4)
const colors = copyOriginalColors()

const { availableColors, coloredStatus } = collectColoredStatus(
const collectedColoredStatus = (experiments: Experiment[]) =>
collectColoredStatus(
experiments,
new Map(),
new Map(),
{},
copyOriginalColors()
)

it('should set new experiments to selected if there are less than 7', () => {
const experiments = buildMockExperiments(4)
const colors = copyOriginalColors()

const { availableColors, coloredStatus } =
collectedColoredStatus(experiments)

expect(availableColors).toStrictEqual(colors.slice(4))
expect(coloredStatus).toStrictEqual({
exp1: colors[0],
@@ -40,12 +45,8 @@ describe('collectColoredStatus', () => {
] as Experiment[]
const colors = copyOriginalColors()

const { availableColors, coloredStatus } = collectColoredStatus(
experiments,
new Map(),
{},
copyOriginalColors()
)
const { availableColors, coloredStatus } =
collectedColoredStatus(experiments)

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

const { availableColors, coloredStatus } = collectColoredStatus(
experiments,
new Map(),
{},
copyOriginalColors()
)
const { availableColors, coloredStatus } =
collectedColoredStatus(experiments)

expect(availableColors).toStrictEqual([])
expect(coloredStatus).toStrictEqual({
@@ -84,6 +81,7 @@ describe('collectColoredStatus', () => {
const { availableColors, coloredStatus } = collectColoredStatus(
experiments,
new Map(),
new Map(),
{
exp2: 0,
exp3: colors[2],
@@ -107,6 +105,7 @@ describe('collectColoredStatus', () => {
const { availableColors, coloredStatus } = collectColoredStatus(
experiments,
new Map(),
new Map(),
{
exp1: 0,
exp10: colors[0],
@@ -138,6 +137,7 @@ describe('collectColoredStatus', () => {
const { availableColors, coloredStatus } = collectColoredStatus(
experiments,
new Map(),
new Map(),
{ exp9: colors[0] },
copyOriginalColors().slice(1)
)
@@ -163,6 +163,7 @@ describe('collectColoredStatus', () => {
const { availableColors, coloredStatus } = collectColoredStatus(
experiments,
new Map(),
new Map(),
{
exp4: colors[0],
exp5: colors[1],
@@ -198,6 +199,7 @@ describe('collectColoredStatus', () => {
const { availableColors, coloredStatus } = collectColoredStatus(
experiments,
checkpointsByTip,
new Map(),
{},
copyOriginalColors()
)
@@ -231,6 +233,7 @@ describe('collectColoredStatus', () => {
const { availableColors, coloredStatus } = collectColoredStatus(
experiments,
checkpointsByTip,
new Map(),
{
checkC1: colors[1],
checkD2: colors[2],
15 changes: 13 additions & 2 deletions extension/src/experiments/model/status/collect.ts
Original file line number Diff line number Diff line change
@@ -25,20 +25,24 @@ const collectStatus = (
if (!id || queued || hasKey(acc, id)) {
return
}

acc[id] = getStatus(acc, unassignColors)
}

const collectExistingStatuses = (
experiments: Experiment[],
checkpointsByTip: Map<string, Experiment[]>,
experimentsByBranch: Map<string, Experiment[]>,
previousStatus: ColoredStatus
) => {
const existingStatuses: ColoredStatus = {}
for (const experiment of [
...experiments,
...flattenMapValues(experimentsByBranch),
...flattenMapValues(checkpointsByTip)
]) {
const { id } = experiment

if (!hasKey(previousStatus, id)) {
continue
}
@@ -70,22 +74,29 @@ export const unassignColors = (
export const collectColoredStatus = (
experiments: Experiment[],
checkpointsByTip: Map<string, Experiment[]>,
experimentsByBranch: Map<string, Experiment[]>,
previousStatus: ColoredStatus,
unassignedColors: Color[]
): { coloredStatus: ColoredStatus; availableColors: Color[] } => {
const flattenExperimentsByBranch = flattenMapValues(experimentsByBranch)
const availableColors = unassignColors(
[...experiments, ...flattenMapValues(checkpointsByTip)],
[
...experiments,
...flattenExperimentsByBranch,
...flattenMapValues(checkpointsByTip)
],
previousStatus,
unassignedColors
)

const coloredStatus = collectExistingStatuses(
experiments,
checkpointsByTip,
experimentsByBranch,
previousStatus
)

for (const experiment of experiments) {
for (const experiment of [...experiments, ...flattenExperimentsByBranch]) {
collectStatus(coloredStatus, experiment, availableColors)

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

0 comments on commit ccd6cab

Please sign in to comment.