Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Call exp show with multiple branches #3827

Merged
merged 15 commits into from
May 16, 2023
Prev Previous commit
Next Next commit
Fix workspace experiment branch
Stephanie Roy committed May 5, 2023
commit 438aa5087f814efa920bed15ac1a536eb621acb0
4 changes: 2 additions & 2 deletions extension/src/cli/dvc/contract.ts
Original file line number Diff line number Diff line change
@@ -90,13 +90,13 @@ export type ExecutorState = {
export type ExpWithError = {
rev: string
name?: string
branch?: string
branch: string
} & DvcError

type ExpWithData = {
rev: string
name?: string
branch?: string
branch: string
data: ExpData
}

4 changes: 3 additions & 1 deletion extension/src/cli/dvc/reader.ts
Original file line number Diff line number Diff line change
@@ -74,7 +74,9 @@ export class DvcReader extends DvcCli {
Flag.JSON
)
if (isDvcError(output) || isEmpty(output)) {
return [{ rev: EXPERIMENT_WORKSPACE_ID, ...(output as DvcError) }]
return [
{ branch: '', rev: EXPERIMENT_WORKSPACE_ID, ...(output as DvcError) }
]
}
return output
}
16 changes: 15 additions & 1 deletion extension/src/cli/git/reader.ts
Original file line number Diff line number Diff line change
@@ -8,6 +8,7 @@ import { isDirectory } from '../../fileSystem'

export const autoRegisteredCommands = {
GIT_GET_BRANCHES: 'getBranches',
GIT_GET_CURRENT_BRANCH: 'getCurrentBranch',
GIT_GET_COMMIT_MESSAGES: 'getCommitMessages',
GIT_GET_NUM_COMMITS: 'getNumCommits',
GIT_GET_REMOTE_URL: 'getRemoteUrl',
@@ -94,13 +95,26 @@ export class GitReader extends GitCli {
const options = getOptions(cwd, Command.BRANCH, Flag.NO_MERGE)
try {
const branches = await this.executeProcess(options)
const test = trimAndSplit(branches)
return trimAndSplit(branches)
} catch {
return []
}
}

public async getCurrentBranch(cwd: string): Promise<string> {
const options = getOptions(cwd, Command.BRANCH)
try {
const branches = await this.executeProcess(options)
const currentBranch = trimAndSplit(branches)[0]
.replace('* (', '')
.replace(')', '')
.replace('HEAD detached at ', '')
return currentBranch
} catch {
return ''
}
}

private async getUntrackedDirectories(cwd: string): Promise<string[]> {
const options = getOptions(
cwd,
1 change: 1 addition & 0 deletions extension/src/experiments/columns/collect/index.test.ts
Original file line number Diff line number Diff line change
@@ -733,6 +733,7 @@ describe('collectRelativeMetricsFiles', () => {
expect(
collectRelativeMetricsFiles([
{
branch: 'main',
rev: EXPERIMENT_WORKSPACE_ID,
error: { msg: 'I broke', type: 'not important' }
}
1 change: 1 addition & 0 deletions extension/src/experiments/data/collect.test.ts
Original file line number Diff line number Diff line change
@@ -16,6 +16,7 @@ describe('collectFiles', () => {
it('should handle an error being returned', () => {
const workspaceOnly = [
{
branch: 'main',
error: { msg: 'bad things are happening', type: 'today' },
rev: EXPERIMENT_WORKSPACE_ID
}
23 changes: 19 additions & 4 deletions extension/src/experiments/data/index.ts
Original file line number Diff line number Diff line change
@@ -47,7 +47,6 @@ export class ExperimentsData extends BaseData<ExpShowOutput> {
}

private async expShow(flags: (ExperimentFlag | string)[], branch?: string) {
// TODO if !branch get current branch
const data = await this.internalCommands.executeCommand<ExpShowOutput>(
AvailableCommands.EXP_SHOW,
this.dvcRoot,
@@ -59,10 +58,23 @@ export class ExperimentsData extends BaseData<ExpShowOutput> {
public async update(): Promise<void> {
let data: ExpShowOutput = []
const isBranchesView = this.experiments.getIsBranchesView()
const branches = this.experiments.getBranchesToShow()

if (isBranchesView) {
data = await this.expShow([ExperimentFlag.ALL_BRANCHES])
data = (await this.expShow([
ExperimentFlag.ALL_BRANCHES
])) as ExpShowOutput
} else {
const currentBranch = await this.internalCommands.executeCommand<string>(
AvailableCommands.GIT_GET_CURRENT_BRANCH,
this.dvcRoot
)
let branches = this.experiments.getBranchesToShow()

if (!branches.includes(currentBranch)) {
branches.push(currentBranch)
this.experiments.setBranchesToShow(branches)
}

const output = []
for (const branch of branches) {
const branchFlags = [
@@ -73,7 +85,10 @@ export class ExperimentsData extends BaseData<ExpShowOutput> {
]
output.push(
new Promise<void>(async resolve => {
const newData = await this.expShow(branchFlags, branch)
const newData = (await this.expShow(
branchFlags,
branch
)) as ExpShowOutput
data.push(...newData)
resolve()
})
13 changes: 10 additions & 3 deletions extension/src/experiments/model/collect.ts
Original file line number Diff line number Diff line change
@@ -163,7 +163,6 @@ const addCommitData = (
}
baseline.description = formatCommitMessage(commit.message)
baseline.commit = commit
baseline.branch = 'a'
}

const collectExpState = (
@@ -172,13 +171,16 @@ const collectExpState = (
commitData: { [sha: string]: CommitData }
): Experiment | undefined => {
const { rev, name, branch } = expState
if (rev === EXPERIMENT_WORKSPACE_ID && acc.workspace) {
return
}
const label =
rev === EXPERIMENT_WORKSPACE_ID
? EXPERIMENT_WORKSPACE_ID
: name || shortenForLabel(rev)
const id = name || label

const experiment: Experiment = { id, label }
const experiment: Experiment = { id, label } as unknown as Experiment
sroy3 marked this conversation as resolved.
Show resolved Hide resolved

const baseline = transformExpState(experiment, expState)
baseline.branch = branch
@@ -247,6 +249,7 @@ const collectExpRange = (

const experiment = transformExpState(
{
branch: 'main',
id: name || label,
label
},
@@ -307,7 +310,11 @@ export const collectExperiments = (
experimentsByCommit: new Map(),
hasCheckpoints: hasCheckpoints(output),
runningExperiments: [],
workspace: { id: EXPERIMENT_WORKSPACE_ID, label: EXPERIMENT_WORKSPACE_ID }
workspace: {
branch: 'main',
id: EXPERIMENT_WORKSPACE_ID,
label: EXPERIMENT_WORKSPACE_ID
}
}

const commitData = getCommitData(commitsOutput)
5 changes: 4 additions & 1 deletion extension/src/experiments/model/index.ts
Original file line number Diff line number Diff line change
@@ -90,6 +90,10 @@ export class ExperimentsModel extends ModelWithPersistence {
{}
)

if (typeof this.numberOfCommitsToShow === 'number') {
this.numberOfCommitsToShow = {}
}

this.branchesToShow = this.revive<string[]>(
PersistenceKey.EXPERIMENTS_BRANCHES,
[]
@@ -384,7 +388,6 @@ export class ExperimentsModel extends ModelWithPersistence {

public setNbfCommitsToShow(numberOfCommitsToShow: number, branch: string) {
this.numberOfCommitsToShow[branch] = numberOfCommitsToShow
debugger
this.persistNbOfCommitsToShow()
}

12 changes: 12 additions & 0 deletions extension/src/experiments/model/quickPick.test.ts
Original file line number Diff line number Diff line change
@@ -91,6 +91,7 @@ describe('pickExperimentsToPlot', () => {

it('should fill the quick pick item details with column values', async () => {
const selectedExperiment = {
branch: 'main',
Created: '2022-08-19T08:17:22',
deps: {
'data/data.xml': { changes: false, value: '22a1a29' }
@@ -184,13 +185,15 @@ describe('pickExperimentsToPlot', () => {
})

const mockedExp1 = {
branch: 'main',
description: '[exp-0580a]',
id: 'abcdefb',
label: 'abcdefb',
name: 'exp-0580a'
}

const mockedExp2 = {
branch: 'main',
description: '[exp-c54c4]',
id: 'abcdefa',
label: 'abcdefa',
@@ -201,54 +204,63 @@ const mockedExpList = [
mockedExp1,
mockedExp2,
{
branch: 'main',
description: '[exp-054f1]',
id: 'abcdef1',
label: 'abcdef1',
name: 'exp-054f1'
},
{
branch: 'main',
description: '[exp-ae4fa]',
id: 'abcdef2',
label: 'abcdef2',
name: 'exp-ae4fa'
},
{
branch: 'main',
description: '[exp-1324e]',
id: 'abcdef3',
label: 'abcdef3',
name: 'exp-1324e'
},
{
branch: 'main',
description: '[exp-3bd24]',
id: 'abcdef4',
label: 'abcdef4',
name: 'exp-3bd24'
},
{
branch: 'main',
description: '[exp-5d170]',
id: 'abcdef5',
label: 'abcdef5',
name: 'exp-5d170'
},
{
branch: 'main',
description: '[exp-9fe22]',
id: 'abcdef6',
label: 'abcdef6',
name: 'exp-9fe22'
},
{
branch: 'main',
description: '[exp-b707b]',
id: 'abcdef7',
label: 'abcdef7',
name: 'exp-b707b'
},
{
branch: 'main',
description: '[exp-47694]',
id: 'abcdef8',
label: 'abcdef8',
name: 'exp-47694'
},
{
branch: 'main',
description: '[exp-59807]',
id: 'abcdef9',
label: 'abcdef9',
1 change: 1 addition & 0 deletions extension/src/experiments/model/sortBy/index.test.ts
Original file line number Diff line number Diff line change
@@ -8,6 +8,7 @@ describe('sortExperiments', () => {
const testLabel = 'f0778b3'
const testTimestamp = '2021-01-14T10:57:59'
const irrelevantExperimentData = {
branch: 'main',
checkpoint_parent: 'f81f1b5a1248b9d9f595fb53136298c69f908e66',
checkpoint_tip: 'd3f4a0d3661c5977540d2205d819470cf0d2145a',
id: testId,
7 changes: 6 additions & 1 deletion extension/src/experiments/model/status/index.test.ts
Original file line number Diff line number Diff line change
@@ -49,7 +49,12 @@ describe('limitToMaxSelected', () => {
expect(
limitToMaxSelected([
...mockedExperiments,
{ id: '1', label: 'R', status: ExperimentStatus.RUNNING }
{
branch: 'main',
id: '1',
label: 'R',
status: ExperimentStatus.RUNNING
}
])
.map(({ label }) => label)
.sort()
9 changes: 7 additions & 2 deletions extension/src/experiments/model/tree.ts
Original file line number Diff line number Diff line change
@@ -31,7 +31,12 @@ import {
} from '../../commands/external'
import { sum } from '../../util/math'
import { Disposable } from '../../class/dispose'
import { Experiment, ExperimentStatus, isRunning } from '../webview/contract'
import {
Commit,
Experiment,
ExperimentStatus,
isRunning
} from '../webview/contract'
import { getMarkdownString } from '../../vscode/markdownString'
import { truncateFromLeft } from '../../util/string'

@@ -119,7 +124,7 @@ export class ExperimentsTree
}

return Promise.resolve(
this.getExperimentsByCommit(element.dvcRoot, element)
this.getExperimentsByCommit(element.dvcRoot, element as unknown as Commit)
)
}

2 changes: 1 addition & 1 deletion extension/src/experiments/webview/contract.ts
Original file line number Diff line number Diff line change
@@ -43,7 +43,7 @@ export type Experiment = {
starred?: boolean
status?: ExperimentStatus
timestamp?: string | null
branch?: string
branch: string
}

export const isRunning = (status: ExperimentStatus | undefined): boolean =>
8 changes: 6 additions & 2 deletions extension/src/experiments/webview/messages.ts
Original file line number Diff line number Diff line change
@@ -98,7 +98,10 @@ export class WebviewMessages {
update && this.sendWebviewMessage()
}

public sendWebviewMessage() {
public sendWebviewMessage(doNotCheckNbCommits?: boolean) {
if (!doNotCheckNbCommits) {
this.changeHasMoreOrLessCommits(true)
}
const webview = this.getWebview()
void webview?.show(this.getWebviewData())
}
@@ -237,6 +240,7 @@ export class WebviewMessages {
}
this.experiments.setBranchesToShow(selectedBranches)
await this.update()
await this.changeHasMoreOrLessCommits(true)
}

private async switchToBranchesView() {
@@ -258,7 +262,7 @@ export class WebviewMessages {
Math.min(nbOfCommitsToShow, availableNbCommits) > 1
}

update && this.sendWebviewMessage()
update && this.sendWebviewMessage(true)
}

private async changeCommitsToShow(change: 1 | -1, branch: string) {
15 changes: 8 additions & 7 deletions extension/src/experiments/workspace.ts
Original file line number Diff line number Diff line change
@@ -440,13 +440,14 @@ export class WorkspaceExperiments extends BaseWorkspaceWebviews<
cwd
)
return await quickPickManyValues(
allBranches
.filter(branch => branch.indexOf('*') !== 0)
.map(branch => ({
label: branch,
picked: branchesSelected.includes(branch),
value: branch
})),
allBranches.map(branch => {
const branchName = branch.replace('* ', '')
return {
label: branchName,
picked: branchesSelected.includes(branchName),
value: branchName
}
}),
{
title: Title.SELECT_BRANCHES
}
Loading