diff --git a/.vscode/settings.json b/.vscode/settings.json index a2e2a3f88d..a44b1284d9 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -15,6 +15,7 @@ "conda", "datapoint", "datapoints", + "Decoratable", "dvcignore", "DVCLIVE", "DVCPATH", diff --git a/extension/src/experiments/columns/extract.test.ts b/extension/src/experiments/columns/extract.test.ts new file mode 100644 index 0000000000..7181a398e4 --- /dev/null +++ b/extension/src/experiments/columns/extract.test.ts @@ -0,0 +1,21 @@ +import { extractColumns } from './extract' + +describe('extractColumns', () => { + it('should handle concatenating errors', () => { + const data = { + metrics: { + 'summary.json': { + error: { msg: 'metrics file is busted', type: 'fatal' } + } + }, + params: { + 'params.yaml': { + error: { msg: 'this is also broken', type: 'impossible' } + } + } + } + + const { error } = extractColumns(data) + expect(error).toStrictEqual('metrics file is busted\nthis is also broken') + }) +}) diff --git a/extension/src/experiments/columns/extract.ts b/extension/src/experiments/columns/extract.ts index 352bf2f776..75bb5b4de2 100644 --- a/extension/src/experiments/columns/extract.ts +++ b/extension/src/experiments/columns/extract.ts @@ -1,4 +1,9 @@ -import { Deps, ExperimentFields, ValueTreeRoot } from '../../cli/reader' +import { + Deps, + ExperimentFields, + ValueTreeOrError, + ValueTreeRoot +} from '../../cli/reader' import { shortenForLabel } from '../../util/string' import { DepColumns, @@ -6,20 +11,35 @@ import { MetricOrParamColumns } from '../webview/contract' +const extractFileMetricsOrParams = ( + acc: { columns: MetricOrParamColumns; errors: string[] }, + file: string, + dataOrError: ValueTreeOrError +) => { + const data = dataOrError?.data + const error = dataOrError?.error?.msg + if (error) { + acc.errors.push(error) + } + if (!data) { + return + } + acc.columns[file] = data +} + const extractMetricsOrParams = ( - columns?: ValueTreeRoot -): MetricOrParamColumns | undefined => { - if (!columns) { + valueTreeRoot?: ValueTreeRoot +): { columns: MetricOrParamColumns; errors: string[] } | undefined => { + if (!valueTreeRoot) { return } - const acc: MetricOrParamColumns = {} + const acc: { columns: MetricOrParamColumns; errors: string[] } = { + columns: {}, + errors: [] + } - for (const [file, dataOrError] of Object.entries(columns)) { - const data = dataOrError?.data - if (!data) { - continue - } - acc[file] = data + for (const [file, dataOrError] of Object.entries(valueTreeRoot)) { + extractFileMetricsOrParams(acc, file, dataOrError) } return acc @@ -46,15 +66,34 @@ const extractDeps = ( return acc } -export const extractColumns = ( - experiment: ExperimentFields, - branch?: Experiment -): { +type Columns = { + error?: string deps: DepColumns | undefined metrics: MetricOrParamColumns | undefined params: MetricOrParamColumns | undefined -} => ({ - deps: extractDeps(experiment.deps, branch), - metrics: extractMetricsOrParams(experiment.metrics), - params: extractMetricsOrParams(experiment.params) -}) +} + +export const extractColumns = ( + experiment: ExperimentFields, + branch?: Experiment +): Columns => { + const metricsData = extractMetricsOrParams(experiment.metrics) + const paramsData = extractMetricsOrParams(experiment.params) + + const error = [ + ...(metricsData?.errors || []), + ...(paramsData?.errors || []) + ].join('\n') + + const columns: Columns = { + deps: extractDeps(experiment.deps, branch), + metrics: metricsData?.columns, + params: paramsData?.columns + } + + if (error) { + columns.error = error + } + + return columns +} diff --git a/extension/src/experiments/index.ts b/extension/src/experiments/index.ts index 285783bf2a..b35ebc5e65 100644 --- a/extension/src/experiments/index.ts +++ b/extension/src/experiments/index.ts @@ -16,7 +16,7 @@ import { ExperimentsData } from './data' import { askToDisableAutoApplyFilters } from './toast' import { Experiment, ColumnType, TableData } from './webview/contract' import { WebviewMessages } from './webview/messages' -import { DecorationProvider } from './model/filterBy/decorationProvider' +import { DecorationProvider } from './model/decorationProvider' import { ResourceLocator } from '../resourceLocator' import { AvailableCommands, InternalCommands } from '../commands/internal' import { ExperimentsOutput } from '../cli/reader' @@ -425,7 +425,8 @@ export class Experiments extends BaseRepository { private notifyChanged(data?: ExperimentsOutput) { this.decorationProvider.setState( this.experiments.getLabels(), - this.experiments.getLabelsToDecorate() + this.experiments.getLabelsToDecorate(), + this.experiments.getErrors() ) this.experimentsChanged.fire(data) this.notifyColumnsChanged() diff --git a/extension/src/experiments/model/collect.ts b/extension/src/experiments/model/collect.ts index 929009909d..73ff5cd987 100644 --- a/extension/src/experiments/model/collect.ts +++ b/extension/src/experiments/model/collect.ts @@ -1,4 +1,9 @@ -import { ThemeIcon, TreeItemCollapsibleState, Uri } from 'vscode' +import { + MarkdownString, + ThemeIcon, + TreeItemCollapsibleState, + Uri +} from 'vscode' import omit from 'lodash.omit' import { ExperimentType } from '.' import { ExperimentsAccumulator } from './accumulator' @@ -29,6 +34,7 @@ export type ExperimentItem = { collapsibleState: TreeItemCollapsibleState type: ExperimentType iconPath: ThemeIcon | Uri | Resource + tooltip: MarkdownString | undefined } type ExperimentsObject = { [sha: string]: ExperimentFieldsOrError } @@ -40,8 +46,12 @@ export const isCheckpoint = ( const getExperimentId = ( sha: string, - experimentsFields: ExperimentFields + experimentsFields?: ExperimentFields ): string => { + if (!experimentsFields) { + return sha + } + const { name, checkpoint_tip } = experimentsFields if (isCheckpoint(checkpoint_tip, sha)) { @@ -115,7 +125,10 @@ const transformColumns = ( experimentFields: ExperimentFields, branch?: Experiment ) => { - const { metrics, params, deps } = extractColumns(experimentFields, branch) + const { error, metrics, params, deps } = extractColumns( + experimentFields, + branch + ) if (metrics) { experiment.metrics = metrics @@ -126,21 +139,28 @@ const transformColumns = ( if (deps) { experiment.deps = deps } + if (error) { + experiment.error = error + } } const transformExperimentData = ( id: string, - experimentFields: ExperimentFields, + experimentFieldsOrError: ExperimentFieldsOrError, label: string | undefined, sha?: string, displayNameOrParent?: string, logicalGroupName?: string, branch?: Experiment ): Experiment => { + const data = experimentFieldsOrError.data || {} + + const error = experimentFieldsOrError.error + const experiment = { id, label, - ...omit(experimentFields, Object.values(ColumnType)) + ...omit(data, Object.values(ColumnType)) } as Experiment if (displayNameOrParent) { @@ -155,7 +175,11 @@ const transformExperimentData = ( experiment.sha = sha } - transformColumns(experiment, experimentFields, branch) + if (error) { + experiment.error = error.msg + } + + transformColumns(experiment, data, branch) return experiment } @@ -172,7 +196,8 @@ const transformExperimentOrCheckpointData = ( } => { const experimentFields = experimentData.data if (!experimentFields) { - return { experiment: undefined } + const error = experimentData?.error?.msg + return { experiment: { error, id: sha, label: shortenForLabel(sha) } } } const checkpointTipId = getCheckpointTipId( @@ -186,7 +211,7 @@ const transformExperimentOrCheckpointData = ( checkpointTipId, experiment: transformExperimentData( id, - experimentFields, + experimentData, shortenForLabel(sha), sha, getDisplayNameOrParent(sha, branchSha, experimentsObject), @@ -254,14 +279,8 @@ const collectFromBranchesObject = ( for (const [sha, { baseline, ...experimentsObject }] of Object.entries( branchesObject )) { - const experimentFields = baseline.data - if (!experimentFields) { - continue - } - - const name = experimentFields.name as string - - const branch = transformExperimentData(name, experimentFields, name, sha) + const name = baseline.data?.name || sha + const branch = transformExperimentData(name, baseline, name, sha) if (branch) { collectFromExperimentsObject(acc, experimentsObject, sha, branch) @@ -278,10 +297,11 @@ export const collectExperiments = ( const { workspace, ...branchesObject } = data const workspaceId = 'workspace' - const workspaceFields = workspace.baseline.data - const workspaceBaseline = workspaceFields - ? transformExperimentData(workspaceId, workspaceFields, workspaceId) - : undefined + const workspaceBaseline = transformExperimentData( + workspaceId, + workspace.baseline, + workspaceId + ) const acc = new ExperimentsAccumulator(workspaceBaseline) diff --git a/extension/src/experiments/model/filterBy/decorationProvider.ts b/extension/src/experiments/model/decorationProvider.ts similarity index 76% rename from extension/src/experiments/model/filterBy/decorationProvider.ts rename to extension/src/experiments/model/decorationProvider.ts index ed8f9e20b3..6f02bb62bd 100644 --- a/extension/src/experiments/model/filterBy/decorationProvider.ts +++ b/extension/src/experiments/model/decorationProvider.ts @@ -7,7 +7,7 @@ import { Uri, window } from 'vscode' -import { Disposable } from '../../../class/dispose' +import { Disposable } from '../../class/dispose' export const getDecoratableUri = (label: string): Uri => Uri.from({ path: label, scheme: 'dvc.experiments' }) @@ -16,6 +16,10 @@ export class DecorationProvider extends Disposable implements FileDecorationProvider { + private static DecorationError: FileDecoration = { + color: new ThemeColor('errorForeground') + } + private static DecorationFiltered: FileDecoration = { color: new ThemeColor('gitDecoration.ignoredResourceForeground'), tooltip: 'Filtered' @@ -24,6 +28,7 @@ export class DecorationProvider public readonly onDidChangeFileDecorations: Event private readonly decorationsChanged: EventEmitter + private errors = new Set() private filtered = new Set() constructor(decorationsChanged?: EventEmitter) { @@ -38,12 +43,19 @@ export class DecorationProvider } public provideFileDecoration(uri: Uri): FileDecoration | undefined { + if (this.errors.has(uri.fsPath)) { + return DecorationProvider.DecorationError + } if (this.filtered.has(uri.fsPath)) { return DecorationProvider.DecorationFiltered } } - public setState(labels: string[], filtered: Set) { + public setState( + labels: string[], + filtered: Set, + errors: Set + ) { const urisToUpdate: Uri[] = [] for (const label of labels) { @@ -51,6 +63,7 @@ export class DecorationProvider } this.filtered = filtered + this.errors = errors this.decorationsChanged.fire(urisToUpdate) } } diff --git a/extension/src/experiments/model/index.ts b/extension/src/experiments/model/index.ts index c710c3114d..8d2dcb5d2c 100644 --- a/extension/src/experiments/model/index.ts +++ b/extension/src/experiments/model/index.ts @@ -310,6 +310,14 @@ export class ExperimentsModel extends ModelWithPersistence { ] } + public getErrors() { + return new Set( + this.getCombinedList() + .filter(({ error }) => error) + .map(({ label }) => label) + ) + } + public getExperimentsWithCheckpoints(): ExperimentWithCheckpoints[] { return this.getExperiments().map(experiment => { const checkpoints = this.checkpointsByTip diff --git a/extension/src/experiments/model/tree.test.ts b/extension/src/experiments/model/tree.test.ts index 3741d91f1e..e19fa6d10d 100644 --- a/extension/src/experiments/model/tree.test.ts +++ b/extension/src/experiments/model/tree.test.ts @@ -1,12 +1,20 @@ import { join } from 'path' import { Disposable, Disposer } from '@hediet/std/disposable' -import { commands, ThemeIcon, TreeItem, Uri, window } from 'vscode' +import { + commands, + MarkdownString, + ThemeIcon, + TreeItem, + Uri, + window +} from 'vscode' import { ExperimentType } from '.' import { ExperimentsTree } from './tree' -import { getDecoratableUri } from './filterBy/decorationProvider' +import { getDecoratableUri } from './decorationProvider' import { buildMockedExperiments } from '../../test/util/jest' import { ResourceLocator } from '../../resourceLocator' import { RegisteredCommands } from '../../commands/external' +import { getMarkdownString } from '../../vscode/markdownString' const mockedCommands = jest.mocked(commands) mockedCommands.registerCommand = jest.fn() @@ -19,6 +27,8 @@ const mockedThemeIcon = jest.mocked(ThemeIcon) const mockedDisposable = jest.mocked(Disposable) +const mockedGetMarkdownString = jest.mocked(getMarkdownString) + const { mockedExperiments, mockedGetDvcRoots, @@ -38,6 +48,7 @@ const mockedResourceLocator = { } as unknown as ResourceLocator jest.mock('vscode') +jest.mock('../../vscode/markdownString') jest.mock('@hediet/std/disposable') beforeEach(() => { @@ -111,6 +122,9 @@ describe('ExperimentsTree', () => { Uri.file(join('path', 'to', 'resources', `${name}-${color}.svg`)) mockedGetExperimentsResource.mockImplementation(getMockedUri) + mockedGetMarkdownString.mockImplementationOnce( + str => str as unknown as MarkdownString + ) const experiments = [ { @@ -131,7 +145,7 @@ describe('ExperimentsTree', () => { type: ExperimentType.EXPERIMENT }, { - displayColor: '#4063e2', + displayColor: undefined, hasChildren: false, id: 'exp-abcdef', label: 'e350702', @@ -139,6 +153,17 @@ describe('ExperimentsTree', () => { selected: false, type: ExperimentType.EXPERIMENT }, + { + displayColor: undefined, + error: + "unable to read: 'params.yaml', YAML file structure is corrupted", + hasChildren: false, + id: '139eabc', + label: '139eabc', + running: false, + selected: false, + type: ExperimentType.EXPERIMENT + }, { hasChildren: false, id: 'f81f1b5', @@ -172,6 +197,7 @@ describe('ExperimentsTree', () => { iconPath: getMockedUri('circle-filled', '#b180d7'), id: 'exp-12345', label: '90aea7f', + tooltip: undefined, type: ExperimentType.EXPERIMENT }, { @@ -186,6 +212,7 @@ describe('ExperimentsTree', () => { iconPath: getMockedUri('loading-spin', '#1a1c19'), id: 'exp-67899', label: 'f0778b3', + tooltip: undefined, type: ExperimentType.EXPERIMENT }, { @@ -197,9 +224,26 @@ describe('ExperimentsTree', () => { }, description: undefined, dvcRoot: 'repo', - iconPath: getMockedUri('circle-outline', '#4063e2'), + iconPath: { id: 'circle-outline' }, id: 'exp-abcdef', label: 'e350702', + tooltip: undefined, + type: ExperimentType.EXPERIMENT + }, + { + collapsibleState: 0, + command: { + arguments: [{ dvcRoot: 'repo', id: '139eabc' }], + command: RegisteredCommands.EXPERIMENT_TOGGLE, + title: 'toggle' + }, + description: undefined, + dvcRoot: 'repo', + iconPath: { id: 'circle-outline' }, + id: '139eabc', + label: '139eabc', + tooltip: + "$(error) unable to read: 'params.yaml', YAML file structure is corrupted", type: ExperimentType.EXPERIMENT }, { @@ -214,6 +258,7 @@ describe('ExperimentsTree', () => { iconPath: mockedClockResource, id: 'f81f1b5', label: 'f81f1b5', + tooltip: undefined, type: ExperimentType.QUEUED } ]) @@ -233,11 +278,13 @@ describe('ExperimentsTree', () => { { id: 'aaaaaaaaaaaaaaaaa', label: 'aaaaaaa', + tooltip: undefined, type: ExperimentType.CHECKPOINT }, { id: 'bbbbbbbbbbbbbbbbb', label: 'bbbbbbb', + tooltip: undefined, type: ExperimentType.CHECKPOINT } ] @@ -250,6 +297,7 @@ describe('ExperimentsTree', () => { iconPath: new ThemeIcon('loading~spin'), id: 'ebbd66f', label: 'ebbd66f', + tooltip: undefined, type: ExperimentType.EXPERIMENT }) @@ -266,6 +314,7 @@ describe('ExperimentsTree', () => { iconPath: new ThemeIcon('circle-filled'), id: 'aaaaaaaaaaaaaaaaa', label: 'aaaaaaa', + tooltip: undefined, type: ExperimentType.CHECKPOINT }, { @@ -280,6 +329,7 @@ describe('ExperimentsTree', () => { iconPath: new ThemeIcon('circle-filled'), id: 'bbbbbbbbbbbbbbbbb', label: 'bbbbbbb', + tooltip: undefined, type: ExperimentType.CHECKPOINT } ]) @@ -329,6 +379,7 @@ describe('ExperimentsTree', () => { iconPath: mockedClockResource, id: 'f0778b3', label: 'f0778b3', + tooltip: undefined, type: ExperimentType.QUEUED }) expect(treeItem).toStrictEqual(mockedItem) @@ -361,6 +412,7 @@ describe('ExperimentsTree', () => { iconPath: new ThemeIcon('loading~spin'), id: 'workspace', label: 'workspace', + tooltip: undefined, type: ExperimentType.WORKSPACE }) @@ -393,6 +445,7 @@ describe('ExperimentsTree', () => { iconPath: new ThemeIcon('loading~spin'), id: 'f0778b3', label: 'f0778b3', + tooltip: undefined, type: ExperimentType.EXPERIMENT }) @@ -425,6 +478,7 @@ describe('ExperimentsTree', () => { iconPath: new ThemeIcon('circle-filled'), id: 'f0778b3', label: 'f0778b3', + tooltip: undefined, type: ExperimentType.EXPERIMENT }) expect(treeItem).toStrictEqual({ @@ -456,6 +510,7 @@ describe('ExperimentsTree', () => { iconPath: new ThemeIcon('circle-filled'), id: 'f0998a3', label: 'f0998a3', + tooltip: undefined, type: ExperimentType.EXPERIMENT }) diff --git a/extension/src/experiments/model/tree.ts b/extension/src/experiments/model/tree.ts index d24974fb3c..b55eb6eac2 100644 --- a/extension/src/experiments/model/tree.ts +++ b/extension/src/experiments/model/tree.ts @@ -10,7 +10,7 @@ import { } from 'vscode' import { ExperimentType } from '.' import { collectDeletable, ExperimentItem } from './collect' -import { getDecoratableUri } from './filterBy/decorationProvider' +import { getDecoratableUri } from './decorationProvider' import { MAX_SELECTED_EXPERIMENTS } from './status' import { WorkspaceExperiments } from '../workspace' import { sendViewOpenedTelemetryEvent } from '../../telemetry' @@ -24,6 +24,7 @@ import { } from '../../commands/external' import { sum } from '../../util/math' import { Disposable } from '../../class/dispose' +import { getMarkdownString } from '../../vscode/markdownString' export class ExperimentsTree extends Disposable @@ -76,12 +77,24 @@ export class ExperimentsTree return getRootItem(element) } - const { label, collapsibleState, iconPath, command, description, type } = - element + const { + label, + collapsibleState, + iconPath, + command, + description, + type, + tooltip + } = element const item = new TreeItem(getDecoratableUri(label), collapsibleState) - item.iconPath = iconPath + if (iconPath) { + item.iconPath = iconPath + } item.description = description item.contextValue = type + if (tooltip) { + item.tooltip = tooltip + } if (command) { item.command = command } @@ -165,6 +178,7 @@ export class ExperimentsTree iconPath: this.getExperimentIcon(experiment), id: experiment.id, label: experiment.label, + tooltip: this.getTooltip(experiment.error), type: experiment.type })) } @@ -228,6 +242,7 @@ export class ExperimentsTree ), id: checkpoint.id, label: checkpoint.label, + tooltip: this.getTooltip(checkpoint.error), type: checkpoint.type })) } @@ -280,4 +295,12 @@ export class ExperimentsTree private getSelectedExperimentItems() { return [...this.view.selection] } + + private getTooltip(error: string | undefined) { + if (!error) { + return + } + + return getMarkdownString(`$(error) ${error}`) + } } diff --git a/extension/src/experiments/webview/contract.ts b/extension/src/experiments/webview/contract.ts index 8035b4591a..cd17a6c703 100644 --- a/extension/src/experiments/webview/contract.ts +++ b/extension/src/experiments/webview/contract.ts @@ -16,19 +16,20 @@ export interface DepColumns { } export interface Experiment extends BaseExperimentFields { + deps?: DepColumns + displayColor?: string + displayNameOrParent?: string + error?: string id: string label: string - displayNameOrParent?: string logicalGroupName?: string - params?: MetricOrParamColumns metrics?: MetricOrParamColumns - deps?: DepColumns + mutable?: boolean outs?: MetricOrParamColumns - displayColor?: string + params?: MetricOrParamColumns selected?: boolean - starred?: boolean - mutable?: boolean sha?: string + starred?: boolean } export interface Row extends Experiment { diff --git a/extension/src/test/fixtures/expShow/noErrors.ts b/extension/src/test/fixtures/expShow/noErrors.ts new file mode 100644 index 0000000000..bbc9bf68ef --- /dev/null +++ b/extension/src/test/fixtures/expShow/noErrors.ts @@ -0,0 +1,24 @@ +import { ExperimentFieldsOrError, ExperimentsOutput } from '../../../cli/reader' +import expShowFixture, { errorShas } from './output' + +const excludeErrors = (): ExperimentsOutput => { + const { workspace, ...branchesObject } = expShowFixture + const expShowFixtureWithoutErrors: ExperimentsOutput = { workspace } + + for (const [sha, { baseline, ...experimentsObject }] of Object.entries( + branchesObject + )) { + const experiments: { [sha: string]: ExperimentFieldsOrError } = {} + for (const [sha, experiment] of Object.entries(experimentsObject)) { + if (!errorShas.includes(sha)) { + experiments[sha] = experiment + } + } + expShowFixtureWithoutErrors[sha] = { baseline, ...experiments } + } + return expShowFixtureWithoutErrors +} + +const data = excludeErrors() + +export default data diff --git a/extension/src/test/fixtures/expShow/output.ts b/extension/src/test/fixtures/expShow/output.ts index fc87719265..ef08ce8758 100644 --- a/extension/src/test/fixtures/expShow/output.ts +++ b/extension/src/test/fixtures/expShow/output.ts @@ -1,6 +1,11 @@ import { join } from '../../util/path' import { ExperimentsOutput } from '../../../cli/reader' +export const errorShas = [ + '489fd8bdaa709f7330aac342e051a9431c625481', + 'f0f918662b4f8c47819ca154a23029bf9b47d4f3' +] + const data: ExperimentsOutput = { workspace: { baseline: { @@ -1545,6 +1550,119 @@ const data: ExperimentsOutput = { timestamp: '2020-12-29T15:26:36' } }, + [errorShas[0]]: { + error: { + type: 'YAMLFileCorruptedError', + msg: "unable to read: 'params.yaml', YAML file structure is corrupted" + } + }, + [errorShas[1]]: { + data: { + deps: { + [join('data', 'data.xml')]: { + hash: '22a1a2931c8370d3aeedd7183606fd7f', + size: 14445097, + nfiles: null + }, + [join('src', 'prepare.py')]: { + hash: 'f09ea0c15980b43010257ccb9f0055e2', + size: 1576, + nfiles: null + }, + [join('data', 'prepared')]: { + hash: '153aad06d376b6595932470e459ef42a.dir', + size: 8437363, + nfiles: 2 + }, + [join('src', 'featurization.py')]: { + hash: 'e0265fc22f056a4b86d85c3056bc2894', + size: 2490, + nfiles: null + }, + [join('data', 'features')]: { + hash: 'f35d4cc2c552ac959ae602162b8543f3.dir', + size: 2232588, + nfiles: 2 + }, + [join('src', 'train.py')]: { + hash: 'c3961d777cfbd7727f9fde4851896006', + size: 967, + nfiles: null + }, + 'model.pkl': { + hash: '46865edbf3d62fc5c039dd9d2b0567a4', + size: 1763725, + nfiles: null + }, + [join('src', 'evaluate.py')]: { + hash: '44e714021a65edf881b1716e791d7f59', + size: 2346, + nfiles: null + } + }, + executor: null, + metrics: { + 'summary.json': { + error: { + type: 'JSONFileCorruptedError', + msg: "unable to read: 'summary.json', JSON file structure is corrupted" + } + } + }, + name: 'exp-f13bca', + outs: { + [join('data', 'prepared')]: { + hash: '153aad06d376b6595932470e459ef42a.dir', + size: 8437363, + nfiles: 2, + use_cache: true, + is_data_source: false + }, + [join('data', 'features')]: { + hash: 'f35d4cc2c552ac959ae602162b8543f3.dir', + size: 2232588, + nfiles: 2, + use_cache: true, + is_data_source: false + }, + 'model.pkl': { + hash: '46865edbf3d62fc5c039dd9d2b0567a4', + size: 1763725, + nfiles: null, + use_cache: true, + is_data_source: false + }, + [join('data', 'data.xml')]: { + hash: '22a1a2931c8370d3aeedd7183606fd7f', + size: 14445097, + nfiles: null, + use_cache: true, + is_data_source: true + } + }, + params: { + 'params.yaml': { + data: { + code_names: [0, 1], + epochs: 5, + learning_rate: 2.1e-7, + dvc_logs_dir: 'dvc_logs', + log_file: 'logs.csv', + dropout: 0.124, + process: { threshold: 0.85 } + } + }, + [join('nested', 'params.yaml')]: { + data: { + test: true + } + } + }, + queued: false, + running: false, + timestamp: '2020-12-29T15:26:36' + } + }, '90aea7f2482117a55dfcadcdb901aaa6610fbbc9': { data: { deps: { diff --git a/extension/src/test/fixtures/expShow/rows.ts b/extension/src/test/fixtures/expShow/rows.ts index f22e136e16..3810a90a43 100644 --- a/extension/src/test/fixtures/expShow/rows.ts +++ b/extension/src/test/fixtures/expShow/rows.ts @@ -1294,6 +1294,99 @@ const data: Row[] = [ ], timestamp: '2020-12-29T15:27:02' }, + { + displayColor: colorsList[5], + id: '489fd8bdaa709f7330aac342e051a9431c625481', + label: '489fd8b', + error: + "unable to read: 'params.yaml', YAML file structure is corrupted", + selected: true + }, + { + deps: { + [join('data', 'data.xml')]: valueWithNoChanges( + '22a1a2931c8370d3aeedd7183606fd7f' + ), + [join('src', 'prepare.py')]: valueWithNoChanges( + 'f09ea0c15980b43010257ccb9f0055e2' + ), + [join('data', 'prepared')]: valueWithNoChanges( + '153aad06d376b6595932470e459ef42a.dir' + ), + [join('src', 'featurization.py')]: valueWithNoChanges( + 'e0265fc22f056a4b86d85c3056bc2894' + ), + [join('data', 'features')]: valueWithNoChanges( + 'f35d4cc2c552ac959ae602162b8543f3.dir' + ), + [join('src', 'train.py')]: valueWithNoChanges( + 'c3961d777cfbd7727f9fde4851896006' + ), + 'model.pkl': valueWithNoChanges('46865edbf3d62fc5c039dd9d2b0567a4'), + [join('src', 'evaluate.py')]: valueWithNoChanges( + '44e714021a65edf881b1716e791d7f59' + ) + }, + displayColor: colorsList[6], + displayNameOrParent: '[exp-f13bca]', + executor: null, + id: 'exp-f13bca', + error: + "unable to read: 'summary.json', JSON file structure is corrupted", + label: 'f0f9186', + logicalGroupName: '[exp-f13bca]', + name: 'exp-f13bca', + outs: { + [join('data', 'prepared')]: { + hash: '153aad06d376b6595932470e459ef42a.dir', + size: 8437363, + nfiles: 2, + use_cache: true, + is_data_source: false + }, + [join('data', 'features')]: { + hash: 'f35d4cc2c552ac959ae602162b8543f3.dir', + size: 2232588, + nfiles: 2, + use_cache: true, + is_data_source: false + }, + 'model.pkl': { + hash: '46865edbf3d62fc5c039dd9d2b0567a4', + size: 1763725, + nfiles: null, + use_cache: true, + is_data_source: false + }, + [join('data', 'data.xml')]: { + hash: '22a1a2931c8370d3aeedd7183606fd7f', + size: 14445097, + nfiles: null, + use_cache: true, + is_data_source: true + } + }, + metrics: {}, + params: { + 'params.yaml': { + code_names: [0, 1], + epochs: 5, + learning_rate: 2.1e-7, + dvc_logs_dir: 'dvc_logs', + log_file: 'logs.csv', + dropout: 0.124, + process: { threshold: 0.85 } + }, + [join('nested', 'params.yaml')]: { + test: true + } + }, + queued: false, + running: false, + selected: true, + sha: 'f0f918662b4f8c47819ca154a23029bf9b47d4f3', + timestamp: '2020-12-29T15:26:36' + }, { deps: { [join('data', 'data.xml')]: valueWithNoChanges( diff --git a/extension/src/test/suite/experiments/index.test.ts b/extension/src/test/suite/experiments/index.test.ts index 4cfe5a800a..7abe59c503 100644 --- a/extension/src/test/suite/experiments/index.test.ts +++ b/extension/src/test/suite/experiments/index.test.ts @@ -90,6 +90,8 @@ suite('Experiments Test Suite', () => { '4fb124a', '42b8736', '1ba7bcd', + '489fd8b', + 'f0f9186', '90aea7f' ]) }) @@ -1092,6 +1094,7 @@ suite('Experiments Test Suite', () => { '217312476f8854dda1865450b737eb6bc7a3ba1b': 0, '22e40e1fa3c916ac567f69b85969e3066a91dda4': 0, '23250b33e3d6dd0e136262d1d26a2face031cb03': 0, + '489fd8bdaa709f7330aac342e051a9431c625481': colors[5], '91116c1eae4b79cb1f5ab0312dfd9b3e43608e15': 0, '9523bde67538cf31230efaff2dbc47d38a944ab5': 0, c658f8b14ac819ac2a5ea0449da6c15dbe8eb880: 0, @@ -1099,6 +1102,7 @@ suite('Experiments Test Suite', () => { e821416bfafb4bc28b3e0a8ddb322505b0ad2361: 0, 'exp-83425': colors[4], 'exp-e7a67': colors[2], + 'exp-f13bca': colors[6], main: colors[1], 'test-branch': colors[3], workspace: colors[0] @@ -1189,6 +1193,7 @@ suite('Experiments Test Suite', () => { '217312476f8854dda1865450b737eb6bc7a3ba1b': 0, '22e40e1fa3c916ac567f69b85969e3066a91dda4': 0, '23250b33e3d6dd0e136262d1d26a2face031cb03': 0, + '489fd8bdaa709f7330aac342e051a9431c625481': colors[5], '91116c1eae4b79cb1f5ab0312dfd9b3e43608e15': 0, '9523bde67538cf31230efaff2dbc47d38a944ab5': 0, c658f8b14ac819ac2a5ea0449da6c15dbe8eb880: 0, @@ -1196,6 +1201,7 @@ suite('Experiments Test Suite', () => { e821416bfafb4bc28b3e0a8ddb322505b0ad2361: 0, 'exp-83425': colors[4], 'exp-e7a67': 0, + 'exp-f13bca': colors[6], main: colors[1], 'test-branch': colors[3], workspace: colors[0] @@ -1208,8 +1214,10 @@ suite('Experiments Test Suite', () => { 'experimentsFilterBy:test': filterMapEntries, 'experimentsSortBy:test': sortDefinitions, 'experimentsStatus:test': { + '489fd8bdaa709f7330aac342e051a9431c625481': 0, 'exp-83425': colors[0], 'exp-e7a67': 0, + 'exp-f13bca': 0, 'test-branch': colors[1] } }) diff --git a/extension/src/test/suite/experiments/model/filterBy/tree.test.ts b/extension/src/test/suite/experiments/model/filterBy/tree.test.ts index a9d2c6e20e..b242be59f0 100644 --- a/extension/src/test/suite/experiments/model/filterBy/tree.test.ts +++ b/extension/src/test/suite/experiments/model/filterBy/tree.test.ts @@ -95,7 +95,7 @@ suite('Experiments Filter By Tree Test Suite', () => { ) }) .map(experiment => - experiment.queued + experiment.queued || experiment.error ? experiment : { ...experiment, diff --git a/extension/src/test/suite/experiments/workspace.test.ts b/extension/src/test/suite/experiments/workspace.test.ts index 0f480f8113..688cf91105 100644 --- a/extension/src/test/suite/experiments/workspace.test.ts +++ b/extension/src/test/suite/experiments/workspace.test.ts @@ -541,6 +541,19 @@ suite('Workspace Experiments Test Suite', () => { id: 'exp-83425', name: 'exp-83425' } + }, + { + description: undefined, + label: '489fd8b', + value: { + id: '489fd8bdaa709f7330aac342e051a9431c625481', + name: '489fd8b' + } + }, + { + description: '[exp-f13bca]', + label: 'f0f9186', + value: { id: 'exp-f13bca', name: 'exp-f13bca' } } ], { diff --git a/extension/src/test/suite/plots/index.test.ts b/extension/src/test/suite/plots/index.test.ts index 1f9d67ae8d..c4742e22ed 100644 --- a/extension/src/test/suite/plots/index.test.ts +++ b/extension/src/test/suite/plots/index.test.ts @@ -7,7 +7,7 @@ import { expect } from 'chai' import { restore, spy, stub } from 'sinon' import { buildPlots } from '../plots/util' import { Disposable } from '../../../extension' -import expShowFixture from '../../fixtures/expShow/output' +import expShowFixtureWithoutErrors from '../../fixtures/expShow/noErrors' import checkpointPlotsFixture from '../../fixtures/expShow/checkpointPlots' import plotsDiffFixture from '../../fixtures/plotsDiff/output' import templatePlotsFixture from '../../fixtures/plotsDiff/template' @@ -91,25 +91,28 @@ suite('Plots Test Suite', () => { ) mockPlotsDiff.resetHistory() - const updatedExpShowFixture = merge(cloneDeep(expShowFixture), { - '53c3851f46955fa3e2b8f6e1c52999acc8c9ea77': { - checkpoint: { - data: { - checkpoint_tip: 'experiment', - queued: false, - running: false - } - }, - experiment: { - data: { - checkpoint_tip: 'experiment', - name: 'exp-e1new', - queued: false, - running: true + const updatedExpShowFixture = merge( + cloneDeep(expShowFixtureWithoutErrors), + { + '53c3851f46955fa3e2b8f6e1c52999acc8c9ea77': { + checkpoint: { + data: { + checkpoint_tip: 'experiment', + queued: false, + running: false + } + }, + experiment: { + data: { + checkpoint_tip: 'experiment', + name: 'exp-e1new', + queued: false, + running: true + } } } } - }) + ) const dataUpdateEvent = new Promise(resolve => disposable.track(data.onDidUpdate(() => resolve(undefined))) @@ -135,9 +138,9 @@ suite('Plots Test Suite', () => { const committedExperiment = { baseline: merge( cloneDeep( - expShowFixture['53c3851f46955fa3e2b8f6e1c52999acc8c9ea77'][ - '4fb124aebddb2adf1545030907687fa9a4c80e70' - ] + expShowFixtureWithoutErrors[ + '53c3851f46955fa3e2b8f6e1c52999acc8c9ea77' + ]['4fb124aebddb2adf1545030907687fa9a4c80e70'] ), { data: { name: 'main' } } ) @@ -195,9 +198,9 @@ suite('Plots Test Suite', () => { const differentBranch = { baseline: merge( cloneDeep( - expShowFixture['53c3851f46955fa3e2b8f6e1c52999acc8c9ea77'][ - '4fb124aebddb2adf1545030907687fa9a4c80e70' - ] + expShowFixtureWithoutErrors[ + '53c3851f46955fa3e2b8f6e1c52999acc8c9ea77' + ]['4fb124aebddb2adf1545030907687fa9a4c80e70'] ), { data: { name: 'another-branch' } } ) @@ -243,7 +246,7 @@ suite('Plots Test Suite', () => { disposable.track(data.onDidUpdate(() => resolve(undefined))) ) - experiments.setState(expShowFixture) + experiments.setState(expShowFixtureWithoutErrors) await dataUpdateEvent expect(mockPlotsDiff).to.be.calledTwice diff --git a/extension/src/test/suite/plots/util.ts b/extension/src/test/suite/plots/util.ts index bb24acd9a9..aa20012e02 100644 --- a/extension/src/test/suite/plots/util.ts +++ b/extension/src/test/suite/plots/util.ts @@ -1,7 +1,7 @@ import { Disposer } from '@hediet/std/disposable' import { stub } from 'sinon' import * as FileSystem from '../../../fileSystem' -import expShowFixture from '../../fixtures/expShow/output' +import expShowFixtureWithoutErrors from '../../fixtures/expShow/noErrors' import checkpointPlotsFixture from '../../fixtures/expShow/checkpointPlots' import { Plots } from '../../../plots' import { buildMockMemento, dvcDemoPath } from '../../util' @@ -23,7 +23,7 @@ import { WebviewMessages } from '../../../plots/webview/messages' export const buildPlots = async ( disposer: Disposer, plotsDiff = {}, - expShow = expShowFixture + expShow = expShowFixtureWithoutErrors ) => { const { internalCommands, diff --git a/extension/src/vscode/markdownString.ts b/extension/src/vscode/markdownString.ts new file mode 100644 index 0000000000..0ae17a6051 --- /dev/null +++ b/extension/src/vscode/markdownString.ts @@ -0,0 +1,4 @@ +import { MarkdownString } from 'vscode' + +export const getMarkdownString = (stringWithCodicons: string) => + new MarkdownString(stringWithCodicons, true) diff --git a/webview/src/experiments/components/App.test.tsx b/webview/src/experiments/components/App.test.tsx index 546f0ba282..2a43ba198d 100644 --- a/webview/src/experiments/components/App.test.tsx +++ b/webview/src/experiments/components/App.test.tsx @@ -889,7 +889,7 @@ describe('App', () => { }) const selectedForPlotsIndicator = screen.getByLabelText('selected for plots') - expect(selectedForPlotsIndicator).toHaveTextContent('5') + expect(selectedForPlotsIndicator).toHaveTextContent('7') expect(screen.queryByRole('tooltip')).not.toBeInTheDocument() @@ -898,7 +898,7 @@ describe('App', () => { const tooltip = screen.getByRole('tooltip') expect(tooltip).toHaveTextContent( - '5 Experiments Selected for Plotting (Max 7)' + '7 Experiments Selected for Plotting (Max 7)' ) setTableData({ diff --git a/webview/src/experiments/components/table/Cell.tsx b/webview/src/experiments/components/table/Cell.tsx index ef5cbf9623..a40237e3e6 100644 --- a/webview/src/experiments/components/table/Cell.tsx +++ b/webview/src/experiments/components/table/Cell.tsx @@ -1,6 +1,7 @@ import React from 'react' import cx from 'classnames' import { VSCodeCheckbox } from '@vscode/webview-ui-toolkit/react' +import { ErrorTooltip } from './Errors' import { Indicator, IndicatorWithJustTheCounter } from './Indicators' import styles from './styles.module.scss' import { CellProp, RowProp } from './interfaces' @@ -136,7 +137,7 @@ export const FirstCell: React.FC< > = ({ cell, bulletColor, toggleExperiment, ...rowActionsProps }) => { const { row, isPlaceholder } = cell const { - original: { queued } + original: { error, queued } } = row const { @@ -172,12 +173,14 @@ export const FirstCell: React.FC< {queued && } {isPlaceholder ? null : ( -
- {cell.render('Cell')} -
+ +
+ {cell.render('Cell')} +
+ )} @@ -186,6 +189,7 @@ export const FirstCell: React.FC< export const CellWrapper: React.FC< CellProp & { + error?: string changes?: string[] cellId: string children?: React.ReactNode diff --git a/webview/src/experiments/components/table/Errors.tsx b/webview/src/experiments/components/table/Errors.tsx new file mode 100644 index 0000000000..e87c530872 --- /dev/null +++ b/webview/src/experiments/components/table/Errors.tsx @@ -0,0 +1,22 @@ +import React from 'react' +import styles from './styles.module.scss' +import ErrorIcon from '../../../shared/components/icons/Error' +import Tooltip from '../../../shared/components/tooltip/Tooltip' + +export const ErrorTooltip: React.FC<{ + error?: string + children: React.ReactElement +}> = ({ children, error }) => ( + + + {error} + + } + placement={'bottom'} + disabled={!error} + > + {children} + +) diff --git a/webview/src/experiments/components/table/Row.tsx b/webview/src/experiments/components/table/Row.tsx index 666fdd28b8..9ffdbae6b5 100644 --- a/webview/src/experiments/components/table/Row.tsx +++ b/webview/src/experiments/components/table/Row.tsx @@ -327,7 +327,7 @@ export const RowContent: React.FC< depth, values: { id } } = row - const { displayColor, starred } = original + const { displayColor, error, starred } = original const isWorkspace = id === 'workspace' const changesIfWorkspace = isWorkspace ? changes : undefined const toggleExperiment = () => { @@ -422,6 +422,7 @@ export const RowContent: React.FC< diff --git a/webview/src/experiments/components/table/styles.module.scss b/webview/src/experiments/components/table/styles.module.scss index c9e816a9d5..738a6bfb7b 100644 --- a/webview/src/experiments/components/table/styles.module.scss +++ b/webview/src/experiments/components/table/styles.module.scss @@ -600,6 +600,27 @@ $workspace-row-edge-margin: $edge-padding - $cell-padding; text-overflow: ellipsis; } +.error { + color: $error-color; +} + +.errorIcon { + margin-left: 6px; + margin-top: 3px; +} + +.errorTooltip { + white-space: pre-wrap; + display: flex; + gap: 4px; + font-size: 0.8125rem; + + svg { + min-width: 16px; + min-height: 16px; + } +} + .headerCellWrapper { @extend %headerCellPadding; } diff --git a/webview/src/experiments/util/buildDynamicColumns.tsx b/webview/src/experiments/util/buildDynamicColumns.tsx index 432ecdb38c..42c7ca3688 100644 --- a/webview/src/experiments/util/buildDynamicColumns.tsx +++ b/webview/src/experiments/util/buildDynamicColumns.tsx @@ -1,4 +1,5 @@ import React, { SyntheticEvent } from 'react' +import cx from 'classnames' import get from 'lodash/get' import { Column as TableColumn, @@ -20,6 +21,7 @@ import Tooltip, { import styles from '../components/table/styles.module.scss' import { CopyButton } from '../../shared/components/copyButton/CopyButton' import { OverflowHoverTooltip } from '../components/overflowHoverTooltip/OverflowHoverTooltip' +import { ErrorTooltip } from '../components/table/Errors' export type CellValue = Value | ValueWithChanges @@ -32,12 +34,24 @@ export const cellValue = (raw: CellValue) => export const cellHasChanges = (cellValue: CellValue) => isValueWithChanges(cellValue) ? cellValue.changes : false -const UndefinedCell = ( +const CellContents: React.FC<{ displayValue: string }> = ({ displayValue }) => ( + {displayValue} +) + +const UndefinedCell: React.FC = () => (
- . . . +
) +const ErrorCell: React.FC<{ error: string }> = ({ error }) => ( + +
+ +
+ +) + const CellTooltip: React.FC<{ stringValue: string }> = ({ stringValue }) => { @@ -56,9 +70,19 @@ const CellTooltip: React.FC<{ } const Cell: React.FC> = cell => { - const { value } = cell + const { + value, + row: { + original: { error } + } + } = cell + + if (error && value === undefined) { + return + } + if (value === undefined) { - return UndefinedCell + return } const rawValue = cellValue(value) @@ -83,7 +107,7 @@ const Cell: React.FC> = cell => { className={styles.copyButton} tooltip="Copy cell contents" /> - {displayValue} + ) @@ -104,6 +128,7 @@ const Header: React.FC<{ column: TableColumn }> = ({ const buildAccessor: (valuePath: string[]) => Accessor = pathArray => originalRow => { const value = get(originalRow, pathArray) + if (!Array.isArray(value)) { return value } diff --git a/webview/src/shared/components/icons/Error.tsx b/webview/src/shared/components/icons/Error.tsx new file mode 100644 index 0000000000..308a37bc41 --- /dev/null +++ b/webview/src/shared/components/icons/Error.tsx @@ -0,0 +1,21 @@ +import * as React from 'react' +import { SVGProps } from 'react' + +const SvgError = (props: SVGProps) => ( + + + +) + +export default SvgError diff --git a/webview/src/shared/variables.scss b/webview/src/shared/variables.scss index 7562139282..8425558a32 100644 --- a/webview/src/shared/variables.scss +++ b/webview/src/shared/variables.scss @@ -8,6 +8,7 @@ $metrics-color: var(--vscode-dvc-metrics); $params-color: var(--vscode-dvc-params); $deps-color: var(--vscode-dvc-deps); $changed-color: var(--vscode-dvc-workspaceChanged); +$error-color: var(--vscode-errorForeground); $row-bg-color: $bg-color; $header-fg-color: $fg-color;