Skip to content

Commit

Permalink
self review
Browse files Browse the repository at this point in the history
  • Loading branch information
mattseddon committed Apr 14, 2023
1 parent 8340d74 commit 27dbd15
Show file tree
Hide file tree
Showing 23 changed files with 75 additions and 126 deletions.
2 changes: 1 addition & 1 deletion extension/src/cli/dvc/reader.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ describe('CliReader', () => {
])
})

it('should return the default output if the cli returns an empty object (no commits)', async () => {
it('should return the default output if the cli returns an empty array (no commits)', async () => {
const cwd = __dirname
mockedCreateProcess.mockReturnValueOnce(
getMockedProcess(JSON.stringify({}))
Expand Down
10 changes: 5 additions & 5 deletions extension/src/experiments/columns/collect/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import {
} from '../../../cli/dvc/contract'
import { standardizePath } from '../../../fileSystem/path'
import { timestampColumn } from '../constants'
import { sortCollectedArray } from '../../../util/array'
import { sortCollectedArray, uniqueValues } from '../../../util/array'

const collectFromExperiment = (
acc: ColumnAccumulator,
Expand Down Expand Up @@ -65,7 +65,7 @@ export const collectColumns = (output: ExpShowOutput): Column[] => {
return hasNoData ? [] : columns
}

export const getData = (expState: ExpState): ExpData | undefined => {
export const getExpData = (expState: ExpState): ExpData | undefined => {
if (experimentHasError(expState)) {
return
}
Expand All @@ -80,8 +80,8 @@ export const collectChanges = (output: ExpShowOutput): string[] => {
}

const [workspaceData, baselineData] = output
const workspace = getData(workspaceData)
const baseline = getData(baselineData)
const workspace = getExpData(workspaceData)
const baseline = getExpData(baselineData)

if (!(workspace && baseline)) {
return changes
Expand Down Expand Up @@ -118,5 +118,5 @@ export const collectRelativeMetricsFiles = (
.filter(Boolean)
.sort()

return [...new Set(files)]
return uniqueValues(files)
}
2 changes: 1 addition & 1 deletion extension/src/experiments/columns/extract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ const extractMetricsOrParams = (
}

const extractDeps = (
columns?: Deps | null,
columns: Deps | null,
commit?: Experiment
): DepColumns | undefined => {
if (!columns) {
Expand Down
8 changes: 4 additions & 4 deletions extension/src/experiments/columns/model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,6 @@ export class ColumnsModel extends PathSelectionModel<Column> {
return this.paramsFiles
}

public getRelativeMetricsFiles() {
return this.relativeMetricsFiles
}

public transformAndSet(data: ExpShowOutput) {
return Promise.all([
this.transformAndSetColumns(data),
Expand All @@ -73,6 +69,10 @@ export class ColumnsModel extends PathSelectionModel<Column> {
return this.columnsChanges
}

public getRelativeMetricsFiles() {
return this.relativeMetricsFiles
}

public setColumnOrder(columnOrder: string[]) {
this.columnOrderState = columnOrder
this.persist(
Expand Down
4 changes: 2 additions & 2 deletions extension/src/experiments/data/collect.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { ExpShowOutput } from '../../cli/dvc/contract'
import { uniqueValues } from '../../util/array'
import { getData } from '../columns/collect'
import { getExpData } from '../columns/collect'

export const collectFiles = (
output: ExpShowOutput,
Expand All @@ -12,7 +12,7 @@ export const collectFiles = (

const [workspace] = output

const data = getData(workspace)
const data = getExpData(workspace)

return uniqueValues([
...Object.keys({
Expand Down
4 changes: 2 additions & 2 deletions extension/src/experiments/data/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ export class ExperimentsData extends BaseData<ExpShowOutput> {

this.experiments = experiments

void this.watchExpGitRefs() // drop all refs that aren't HEAD
void this.managedUpdate(QUEUED_EXPERIMENT_PATH) // can also drop full vs partial updates (https://github.com/iterative/dvc/pull/9170#issuecomment-1467937058)
void this.watchExpGitRefs()
void this.managedUpdate(QUEUED_EXPERIMENT_PATH)
}

public managedUpdate(path?: string) {
Expand Down
8 changes: 0 additions & 8 deletions extension/src/experiments/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -449,14 +449,6 @@ export class Experiments extends BaseRepository<TableData> {
this.experiments.setRevisionCollected(revisions)
}

public getCommitRevisions() {
return this.experiments.getCommitRevisions()
}

public getExperimentRevisions() {
return this.experiments.getExperimentRevisions()
}

public getFinishedExperiments() {
return this.experiments.getFinishedExperiments()
}
Expand Down
22 changes: 0 additions & 22 deletions extension/src/experiments/model/accumulator.ts

This file was deleted.

4 changes: 2 additions & 2 deletions extension/src/experiments/model/collect.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@ describe('collectExperiments', () => {
`61bed4ce8913eca7f73ca754d65bc5daad1520e2\nJohn Smith\n3 days ago\nrefNames:tag: v.1.1\nmessage:add new feature${COMMITS_SEPARATOR}351e42ace3cb6a3a853c65bef285e60748cc6341\nrenovate[bot]\n5 weeks ago\nrefNames:\nmessage:update various dependencies\n* update dvc\n* update dvclive`
)
const [branch1, branch2] = commits
expect(branch1.displayName).toStrictEqual('add new feature')
expect(branch2.displayName).toStrictEqual('update various dependencies ...')
expect(branch1.description).toStrictEqual('add new feature')
expect(branch2.description).toStrictEqual('update various dependencies ...')
expect(branch1.commit).toStrictEqual({
author: 'John Smith',
date: '3 days ago',
Expand Down
18 changes: 12 additions & 6 deletions extension/src/experiments/model/collect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,8 @@ import {
Uri
} from 'vscode'
import { ExperimentType } from '.'
import { ExperimentsAccumulator } from './accumulator'
import { extractColumns } from '../columns/extract'
import { Experiment, CommitData } from '../webview/contract'
import { Experiment, CommitData, RunningExperiment } from '../webview/contract'
import {
EXPERIMENT_WORKSPACE_ID,
ExperimentStatus,
Expand Down Expand Up @@ -41,6 +40,13 @@ export type ExperimentItem = {
tooltip: MarkdownString | undefined
}

type ExperimentsAccumulator = {
commits: Experiment[]
experimentsByCommit: Map<string, Experiment[]>
runningExperiments: RunningExperiment[]
workspace: Experiment
}

const transformColumns = (
experiment: Experiment,
experimentFields: ExpData,
Expand Down Expand Up @@ -154,7 +160,7 @@ const addCommitData = (
if (!commit) {
return
}
baseline.displayName = formatCommitMessage(commit.message)
baseline.description = formatCommitMessage(commit.message)
baseline.commit = commit
}

Expand Down Expand Up @@ -246,9 +252,9 @@ const collectExpRange = (
)

if (name) {
const displayName = `[${name}]`
experiment.displayName = displayName
experiment.logicalGroupName = displayName
const description = `[${name}]`
experiment.description = description
experiment.logicalGroupName = description
}

collectExecutorInfo(acc, experiment, executor)
Expand Down
10 changes: 1 addition & 9 deletions extension/src/experiments/model/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -228,16 +228,8 @@ export class ExperimentsModel extends ModelWithPersistence {
return result
}

public getCommitRevisions() {
return this.commits.map(({ id, sha }) => ({ id, sha }))
}

public getExperimentRevisions() {
return this.getExperiments().map(({ id, label }) => ({ id, label }))
}

public getRevisions() {
return this.getCombinedList().map(({ label }) => label)
return this.getCombinedList().map(({ id }) => id)
}

public getSelectedRevisions() {
Expand Down
34 changes: 17 additions & 17 deletions extension/src/experiments/model/quickPick.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,13 @@ describe('pickExperimentsToPlot', () => {

it('should return the selected experiment', async () => {
const selectedExperiment = {
displayName: '[exp-789]',
description: '[exp-789]',
label: '7c366f6',
selected: false
}
const mockedExperiments = [
{ displayName: '[exp-123]', label: '73de3fe', selected: false },
{ displayName: '[exp-456]', label: '0be657c', selected: true },
{ description: '[exp-123]', label: '73de3fe', selected: false },
{ description: '[exp-456]', label: '0be657c', selected: true },
selectedExperiment
] as Experiment[]

Expand Down Expand Up @@ -95,7 +95,7 @@ describe('pickExperimentsToPlot', () => {
deps: {
'data/data.xml': { changes: false, value: '22a1a29' }
},
displayName: '[exp-123]',
description: '[exp-123]',
id: 'exp-123',
label: '123fsf4',
params: {
Expand All @@ -112,7 +112,7 @@ describe('pickExperimentsToPlot', () => {
deps: {
'data/data.xml': { changes: false, value: '22a1a29' }
},
displayName: '[exp-456]',
description: '[exp-456]',
id: 'exp-456',
label: '456fsf4',
params: {
Expand All @@ -127,7 +127,7 @@ describe('pickExperimentsToPlot', () => {
deps: {
'data/data.xml': { changes: false, value: '22a1a29' }
},
displayName: '[exp-789]',
description: '[exp-789]',
id: 'exp-789',
label: '789fsf4',
params: {
Expand Down Expand Up @@ -184,14 +184,14 @@ describe('pickExperimentsToPlot', () => {
})

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

const mockedExp2 = {
displayName: '[exp-c54c4]',
description: '[exp-c54c4]',
id: 'abcdefa',
label: 'abcdefa',
name: 'exp-c54c4'
Expand All @@ -201,55 +201,55 @@ const mockedExpList = [
mockedExp1,
mockedExp2,
{
displayName: '[exp-054f1]',
description: '[exp-054f1]',
id: 'abcdef1',
label: 'abcdef1',
name: 'exp-054f1'
},
{
displayName: '[exp-ae4fa]',
description: '[exp-ae4fa]',
id: 'abcdef2',
label: 'abcdef2',
name: 'exp-ae4fa'
},
{
displayName: '[exp-1324e]',
description: '[exp-1324e]',
id: 'abcdef3',
label: 'abcdef3',
name: 'exp-1324e'
},
{
displayName: '[exp-3bd24]',
description: '[exp-3bd24]',
id: 'abcdef4',
label: 'abcdef4',
name: 'exp-3bd24'
},
{
displayName: '[exp-5d170]',
description: '[exp-5d170]',
id: 'abcdef5',
label: 'abcdef5',
name: 'exp-5d170'
},
{
displayName: '[exp-9fe22]',
description: '[exp-9fe22]',
id: 'abcdef6',
label: 'abcdef6',
name: 'exp-9fe22'
},
{
displayName: '[exp-b707b]',
description: '[exp-b707b]',
id: 'abcdef7',
label: 'abcdef7',
name: 'exp-b707b'
},
{
displayName: '[exp-47694]',
description: '[exp-47694]',
id: 'abcdef8',
label: 'abcdef8',
name: 'exp-47694'
},
{
displayName: '[exp-59807]',
description: '[exp-59807]',
id: 'abcdef9',
label: 'abcdef9',
name: 'exp-59807'
Expand Down
8 changes: 4 additions & 4 deletions extension/src/experiments/model/quickPick.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ const getItemWithDescription = (
firstThreeColumnOrder: string[]
) => {
const item = getItem(experiment, firstThreeColumnOrder)
if (experiment.displayName) {
if (experiment.description) {
item.description = `${experiment.commit ? '$(git-commit)' : ''}${
experiment.displayName
experiment.description
}`
}
return item
Expand Down Expand Up @@ -104,10 +104,10 @@ const getExperimentItems = (
firstThreeColumnOrder: string[]
): ExperimentItem[] =>
experiments.map(experiment => {
const { label, id, displayName, commit } = experiment
const { label, id, description, commit } = experiment
return {
description:
displayName && `${commit ? '$(git-commit)' : ''}${displayName}`,
description && `${commit ? '$(git-commit)' : ''}${description}`,
detail: getColumnPathsQuickPickDetail(experiment, firstThreeColumnOrder),
label,
value: id
Expand Down
2 changes: 1 addition & 1 deletion extension/src/experiments/model/tree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ export class ExperimentsTree
command: RegisteredCommands.EXPERIMENT_TOGGLE,
title: 'toggle'
},
description: experiment.displayName,
description: experiment.description,
dvcRoot,
iconPath: this.getExperimentIcon(experiment),
id: experiment.id,
Expand Down
2 changes: 1 addition & 1 deletion extension/src/experiments/webview/contract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export type Experiment = {
Created?: string
deps?: DepColumns
displayColor?: string
displayName?: string
description?: string
error?: string
executor?: Executor
id: string
Expand Down
1 change: 0 additions & 1 deletion extension/src/plots/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,6 @@ export class Plots extends BaseRepository<TPlotsData> {
}

private async initializeData() {
await this.plots.transformAndSetExperiments()
this.triggerDataUpdate()
await Promise.all([
this.data.isReady(),
Expand Down
Loading

0 comments on commit 27dbd15

Please sign in to comment.