diff --git a/extension/src/experiments/columns/tree.test.ts b/extension/src/experiments/columns/tree.test.ts index 2378f69711..268f938f3b 100644 --- a/extension/src/experiments/columns/tree.test.ts +++ b/extension/src/experiments/columns/tree.test.ts @@ -124,7 +124,8 @@ describe('ExperimentsColumnsTree', () => { dvcRoot: mockedDvcRoot, iconPath: mockedSelectedCheckbox, label: timestampColumn.label, - path: timestampColumn.path + path: timestampColumn.path, + tooltip: undefined }, { collapsibleState: 1, @@ -132,7 +133,8 @@ describe('ExperimentsColumnsTree', () => { dvcRoot: mockedDvcRoot, iconPath: mockedSelectedCheckbox, label: 'summary.json', - path: buildMetricOrParamPath(ColumnType.METRICS, 'summary.json') + path: buildMetricOrParamPath(ColumnType.METRICS, 'summary.json'), + tooltip: undefined }, { collapsibleState: 1, @@ -140,7 +142,8 @@ describe('ExperimentsColumnsTree', () => { dvcRoot: mockedDvcRoot, iconPath: mockedSelectedCheckbox, label: 'params.yaml', - path: buildMetricOrParamPath(ColumnType.PARAMS, 'params.yaml') + path: buildMetricOrParamPath(ColumnType.PARAMS, 'params.yaml'), + tooltip: undefined }, { collapsibleState: 1, @@ -151,7 +154,8 @@ describe('ExperimentsColumnsTree', () => { path: buildMetricOrParamPath( ColumnType.PARAMS, join('nested', 'params.yaml') - ) + ), + tooltip: undefined }, { collapsibleState: 1, @@ -159,7 +163,8 @@ describe('ExperimentsColumnsTree', () => { dvcRoot: mockedDvcRoot, iconPath: mockedSelectedCheckbox, label: 'data', - path: buildDepPath('data') + path: buildDepPath('data'), + tooltip: undefined }, { collapsibleState: 1, @@ -167,7 +172,8 @@ describe('ExperimentsColumnsTree', () => { dvcRoot: mockedDvcRoot, iconPath: mockedSelectedCheckbox, label: 'src', - path: buildDepPath('src') + path: buildDepPath('src'), + tooltip: undefined }, { collapsibleState: 0, @@ -175,7 +181,8 @@ describe('ExperimentsColumnsTree', () => { dvcRoot: mockedDvcRoot, iconPath: mockedSelectedCheckbox, label: 'model.pkl', - path: buildDepPath('model.pkl') + path: buildDepPath('model.pkl'), + tooltip: undefined } ]) }) @@ -211,7 +218,8 @@ describe('ExperimentsColumnsTree', () => { dvcRoot: mockedDvcRoot, iconPath: mockedSelectedCheckbox, label: timestampColumn.label, - path: timestampColumn.path + path: timestampColumn.path, + tooltip: undefined }, { collapsibleState: 1, @@ -219,7 +227,8 @@ describe('ExperimentsColumnsTree', () => { dvcRoot: mockedDvcRoot, iconPath: mockedSelectedCheckbox, label: 'summary.json', - path: buildMetricOrParamPath(ColumnType.METRICS, 'summary.json') + path: buildMetricOrParamPath(ColumnType.METRICS, 'summary.json'), + tooltip: undefined }, { collapsibleState: 1, @@ -227,7 +236,8 @@ describe('ExperimentsColumnsTree', () => { dvcRoot: mockedDvcRoot, iconPath: mockedSelectedCheckbox, label: 'params.yaml', - path: paramsPath + path: paramsPath, + tooltip: undefined }, { collapsibleState: 1, @@ -238,7 +248,8 @@ describe('ExperimentsColumnsTree', () => { path: buildMetricOrParamPath( ColumnType.PARAMS, join('nested', 'params.yaml') - ) + ), + tooltip: undefined }, { collapsibleState: 1, @@ -246,7 +257,8 @@ describe('ExperimentsColumnsTree', () => { dvcRoot: mockedDvcRoot, iconPath: mockedSelectedCheckbox, label: 'data', - path: buildDepPath('data') + path: buildDepPath('data'), + tooltip: undefined }, { collapsibleState: 1, @@ -254,7 +266,8 @@ describe('ExperimentsColumnsTree', () => { dvcRoot: mockedDvcRoot, iconPath: mockedSelectedCheckbox, label: 'src', - path: buildDepPath('src') + path: buildDepPath('src'), + tooltip: undefined }, { collapsibleState: 0, @@ -262,7 +275,8 @@ describe('ExperimentsColumnsTree', () => { dvcRoot: mockedDvcRoot, iconPath: mockedSelectedCheckbox, label: 'model.pkl', - path: buildDepPath('model.pkl') + path: buildDepPath('model.pkl'), + tooltip: undefined } ]) @@ -276,7 +290,8 @@ describe('ExperimentsColumnsTree', () => { descendantStatuses: [Status.UNSELECTED, Status.SELECTED], hasChildren: true, label: getLabel(param.path), - status: Status.INDETERMINATE + status: Status.INDETERMINATE, + tooltip: undefined } } return { @@ -284,7 +299,8 @@ describe('ExperimentsColumnsTree', () => { descendantStatuses: undefined, hasChildren: false, label: getLabel(param.path), - status: Status.SELECTED + status: Status.SELECTED, + tooltip: undefined } }) ) @@ -295,7 +311,8 @@ describe('ExperimentsColumnsTree', () => { dvcRoot: mockedDvcRoot, iconPath: mockedSelectedCheckbox, label: 'params.yaml', - path: paramsPath + path: paramsPath, + tooltip: undefined }) expect(grandChildren).toStrictEqual([ { @@ -304,7 +321,8 @@ describe('ExperimentsColumnsTree', () => { dvcRoot: mockedDvcRoot, iconPath: mockedSelectedCheckbox, label: 'code_names', - path: appendColumnToPath(paramsPath, 'code_names') + path: appendColumnToPath(paramsPath, 'code_names'), + tooltip: undefined }, { collapsibleState: 0, @@ -312,7 +330,8 @@ describe('ExperimentsColumnsTree', () => { dvcRoot: mockedDvcRoot, iconPath: mockedSelectedCheckbox, label: 'epochs', - path: appendColumnToPath(paramsPath, 'epochs') + path: appendColumnToPath(paramsPath, 'epochs'), + tooltip: undefined }, { collapsibleState: 0, @@ -320,7 +339,8 @@ describe('ExperimentsColumnsTree', () => { dvcRoot: mockedDvcRoot, iconPath: mockedSelectedCheckbox, label: 'learning_rate', - path: appendColumnToPath(paramsPath, 'learning_rate') + path: appendColumnToPath(paramsPath, 'learning_rate'), + tooltip: undefined }, { collapsibleState: 0, @@ -328,7 +348,8 @@ describe('ExperimentsColumnsTree', () => { dvcRoot: mockedDvcRoot, iconPath: mockedSelectedCheckbox, label: 'dvc_logs_dir', - path: appendColumnToPath(paramsPath, 'dvc_logs_dir') + path: appendColumnToPath(paramsPath, 'dvc_logs_dir'), + tooltip: undefined }, { collapsibleState: 0, @@ -336,7 +357,8 @@ describe('ExperimentsColumnsTree', () => { dvcRoot: mockedDvcRoot, iconPath: mockedSelectedCheckbox, label: 'log_file', - path: appendColumnToPath(paramsPath, 'log_file') + path: appendColumnToPath(paramsPath, 'log_file'), + tooltip: undefined }, { collapsibleState: 0, @@ -344,7 +366,8 @@ describe('ExperimentsColumnsTree', () => { dvcRoot: mockedDvcRoot, iconPath: mockedSelectedCheckbox, label: 'dropout', - path: appendColumnToPath(paramsPath, 'dropout') + path: appendColumnToPath(paramsPath, 'dropout'), + tooltip: undefined }, { collapsibleState: 1, @@ -352,7 +375,8 @@ describe('ExperimentsColumnsTree', () => { dvcRoot: mockedDvcRoot, iconPath: mockedIndeterminateCheckbox, label: 'process', - path: processPath + path: processPath, + tooltip: undefined } ]) @@ -364,7 +388,8 @@ describe('ExperimentsColumnsTree', () => { descendantStatuses: undefined, hasChildren: false, label: getLabel(param.path), - status: Status.SELECTED + status: Status.SELECTED, + tooltip: undefined })) ) const greatGrandChildren = await experimentsColumnsTree.getChildren({ @@ -373,7 +398,8 @@ describe('ExperimentsColumnsTree', () => { dvcRoot: mockedDvcRoot, iconPath: mockedIndeterminateCheckbox, label: 'process', - path: processPath + path: processPath, + tooltip: undefined }) expect(greatGrandChildren).toStrictEqual([ @@ -388,7 +414,8 @@ describe('ExperimentsColumnsTree', () => { 'params.yaml', 'process', 'threshold' - ) + ), + tooltip: undefined }, { collapsibleState: 0, @@ -401,7 +428,8 @@ describe('ExperimentsColumnsTree', () => { 'params.yaml', 'process', 'test_arg' - ) + ), + tooltip: undefined } ]) }) @@ -454,7 +482,8 @@ describe('ExperimentsColumnsTree', () => { dvcRoot: mockedDvcRoot, iconPath: mockedSelectedCheckbox, label: filename, - path: relParamsPath + path: relParamsPath, + tooltip: undefined } const treeItem = experimentsColumnsTree.getTreeItem(columnsItem) @@ -494,7 +523,8 @@ describe('ExperimentsColumnsTree', () => { dvcRoot: mockedDvcRoot, iconPath: mockedEmptyCheckbox, label: filename, - path: relParamsPath + path: relParamsPath, + tooltip: undefined } const treeItem = experimentsColumnsTree.getTreeItem(columnsItem) diff --git a/extension/src/path/selection/tree.ts b/extension/src/path/selection/tree.ts index c600fb057f..bad9a396d6 100644 --- a/extension/src/path/selection/tree.ts +++ b/extension/src/path/selection/tree.ts @@ -1,5 +1,6 @@ import { Event, + MarkdownString, TreeDataProvider, TreeItem, TreeItemCollapsibleState, @@ -24,6 +25,7 @@ export type PathSelectionItem = { label: string | undefined path: string iconPath: Resource | Uri + tooltip: MarkdownString | undefined } export abstract class BasePathSelectionTree< @@ -76,7 +78,7 @@ export abstract class BasePathSelectionTree< return new TreeItem(resourceUri, TreeItemCollapsibleState.Collapsed) } - const { dvcRoot, path, description, iconPath } = element + const { dvcRoot, path, description, iconPath, tooltip } = element const treeItem = this.getBaseTreeItem(element) @@ -90,6 +92,9 @@ export abstract class BasePathSelectionTree< if (description) { treeItem.description = description } + if (tooltip) { + treeItem.tooltip = tooltip + } return treeItem } @@ -132,9 +137,17 @@ export abstract class BasePathSelectionTree< path: string status: Status label?: string + tooltip?: MarkdownString }) { - const { dvcRoot, descendantStatuses, hasChildren, path, status, label } = - element + const { + dvcRoot, + descendantStatuses, + hasChildren, + path, + status, + label, + tooltip + } = element const description = this.getDescription(descendantStatuses, '/') const iconPath = this.getIconPath(status) @@ -148,8 +161,9 @@ export abstract class BasePathSelectionTree< dvcRoot, iconPath, label, - path - } as PathSelectionItem + path, + tooltip + } } private updateDescriptionOnChange() { diff --git a/extension/src/plots/errors/collect.test.ts b/extension/src/plots/errors/collect.test.ts index 28183b4320..9e3caa679b 100644 --- a/extension/src/plots/errors/collect.test.ts +++ b/extension/src/plots/errors/collect.test.ts @@ -1,5 +1,9 @@ import { join } from 'path' -import { collectErrors, collectImageErrors } from './collect' +import { + collectErrors, + collectImageErrors, + collectPathErrorsTable +} from './collect' import { EXPERIMENT_WORKSPACE_ID } from '../../cli/dvc/contract' describe('collectErrors', () => { @@ -135,3 +139,154 @@ describe('collectImageErrors', () => { ) }) }) + +describe('collectPathErrorsTable', () => { + it('should return undefined if the errors do not relate to selected revisions', () => { + const rev = 'main' + const path = 'wat' + const markdownTable = collectPathErrorsTable( + path, + [EXPERIMENT_WORKSPACE_ID], + [ + { + msg: '', + name: path, + rev, + source: path, + type: 'FileNotFoundError' + }, + { + msg: 'catastrophic error', + name: path, + rev, + source: path, + type: 'SomeError' + }, + { + msg: '', + name: path, + rev, + source: path, + type: 'UNEXPECTEDERRRRROR' + } + ] + ) + expect(markdownTable).toBeUndefined() + }) + + it('should return undefined if the errors do not relate to the path', () => { + const rev = 'main' + const path = 'wat' + const markdownTable = collectPathErrorsTable( + join('other', 'path'), + [rev], + [ + { + msg: '', + name: path, + rev, + source: path, + type: 'FileNotFoundError' + }, + { + msg: 'catastrophic error', + name: path, + rev, + source: path, + type: 'SomeError' + }, + { + msg: '', + name: path, + rev, + source: path, + type: 'UNEXPECTEDERRRRROR' + } + ] + ) + expect(markdownTable).toBeUndefined() + }) + + it('should construct a markdown table with the error if they relate to the select revision and provided path', () => { + const rev = 'a-really-long-branch-name' + const path = 'wat' + const markdownTable = collectPathErrorsTable( + path, + [EXPERIMENT_WORKSPACE_ID, rev], + [ + { + msg: '', + name: path, + rev: EXPERIMENT_WORKSPACE_ID, + source: path, + type: 'FileNotFoundError' + }, + { + msg: 'catastrophic error', + name: path, + rev, + source: path, + type: 'SomeError' + }, + { + msg: '', + name: path, + rev, + source: path, + type: 'UNEXPECTEDERRRRROR' + } + ] + ) + expect(markdownTable).toStrictEqual( + 'Errors\n' + + '|||\n' + + '|--|--|\n' + + '| a-really... | SomeError: catastrophic error |\n' + + '| a-really... | UNEXPECTEDERRRRROR |\n' + + '| workspace | FileNotFoundError: wat not found. |' + ) + }) + + it('should not duplicate entries in the table', () => { + const name = 'dvc.yaml::Accuracy' + const msg = + "Could not find provided field ('acc_') in data fields ('step, acc')." + const type = 'FieldNotFoundError' + const duplicateEntry = { + msg, + name, + rev: 'aa1401b', + type + } + + const markdownTable = collectPathErrorsTable( + 'dvc.yaml::Accuracy', + [EXPERIMENT_WORKSPACE_ID, 'main', 'test-plots-diff', 'aa1401b'], + [ + { + msg, + name, + rev: 'workspace', + type + }, + { + msg, + name, + rev: 'test-plots-diff', + type + }, + duplicateEntry, + duplicateEntry + ] + ) + + expect(markdownTable).toStrictEqual( + 'Errors\n' + + '|||\n' + + '|--|--|\n' + + "| aa1401b | FieldNotFoundError: Could not find provided field ('acc_') in data fields ('step, acc'). |\n" + + "| test-plo... | FieldNotFoundError: Could not find provided field ('acc_') in data fields ('step, acc'). |\n" + + "| workspace | FieldNotFoundError: Could not find provided field ('acc_') in data fields ('step, acc'). |" + ) + }) +}) diff --git a/extension/src/plots/errors/collect.ts b/extension/src/plots/errors/collect.ts index 0da54d1b58..81c4064833 100644 --- a/extension/src/plots/errors/collect.ts +++ b/extension/src/plots/errors/collect.ts @@ -1,4 +1,9 @@ -import { PlotError, PlotsOutput } from '../../cli/dvc/contract' +import { + EXPERIMENT_WORKSPACE_ID, + PlotError, + PlotsOutput +} from '../../cli/dvc/contract' +import { truncate } from '../../util/string' export const collectErrors = ( data: PlotsOutput, @@ -51,3 +56,36 @@ export const collectImageErrors = ( return msgs.join('\n') } + +const formatError = (acc: string[]): string | undefined => { + if (acc.length === 0) { + return + } + + acc.sort() + acc.unshift('Errors\n|||\n|--|--|') + + return acc.join('\n') +} + +export const collectPathErrorsTable = ( + path: string, + selectedRevisions: string[], + errors: PlotError[] +): string | undefined => { + const acc = new Set() + for (const error of errors) { + const { name, rev } = error + if (name !== path || !selectedRevisions.includes(rev)) { + continue + } + + const row = `| ${truncate( + rev, + EXPERIMENT_WORKSPACE_ID.length + )} | ${getMessage(error)} |` + + acc.add(row) + } + return formatError([...acc]) +} diff --git a/extension/src/plots/errors/decorationProvider.ts b/extension/src/plots/errors/decorationProvider.ts new file mode 100644 index 0000000000..b55749c268 --- /dev/null +++ b/extension/src/plots/errors/decorationProvider.ts @@ -0,0 +1,28 @@ +import { EventEmitter, FileDecoration, Uri } from 'vscode' +import { DecoratableTreeItemScheme, getDecoratableUri } from '../../tree' +import { ErrorDecorationProvider } from '../../tree/errorDecorationProvider' +import { uniqueValues } from '../../util/array' + +export class DecorationProvider extends ErrorDecorationProvider { + private errors = new Set() + + constructor(decorationsChanged?: EventEmitter) { + super(DecoratableTreeItemScheme.PLOTS, decorationsChanged) + } + + public provideFileDecoration(uri: Uri): FileDecoration | undefined { + if (this.errors.has(uri.fsPath)) { + return DecorationProvider.DecorationError + } + } + + public setState(errors: Set = new Set()) { + const urisToUpdate: Uri[] = [] + + for (const label of uniqueValues([...errors, ...this.errors])) { + urisToUpdate.push(getDecoratableUri(label, this.scheme)) + } + this.errors = errors + this.decorationsChanged.fire(urisToUpdate) + } +} diff --git a/extension/src/plots/errors/model.ts b/extension/src/plots/errors/model.ts index accc22990f..ca2a63937f 100644 --- a/extension/src/plots/errors/model.ts +++ b/extension/src/plots/errors/model.ts @@ -1,4 +1,9 @@ -import { collectErrors, collectImageErrors } from './collect' +import { join } from 'path' +import { + collectErrors, + collectImageErrors, + collectPathErrorsTable +} from './collect' import { Disposable } from '../../class/dispose' import { PlotError, PlotsOutputOrError } from '../../cli/dvc/contract' import { isDvcError } from '../../cli/dvc/reader' @@ -28,4 +33,18 @@ export class ErrorsModel extends Disposable { public getImageErrors(path: string, revision: string) { return collectImageErrors(path, revision, this.errors) } + + public getPathErrors(path: string, selectedRevisions: string[]) { + return collectPathErrorsTable(path, selectedRevisions, this.errors) + } + + public getErrorPaths(selectedRevisions: string[]) { + const acc = new Set() + for (const { name, rev } of this.errors) { + if (selectedRevisions.includes(rev)) { + acc.add(join(this.dvcRoot, name)) + } + } + return acc + } } diff --git a/extension/src/plots/index.ts b/extension/src/plots/index.ts index dd55d186ca..71c48e058b 100644 --- a/extension/src/plots/index.ts +++ b/extension/src/plots/index.ts @@ -4,6 +4,7 @@ import { PlotsData as TPlotsData } from './webview/contract' import { WebviewMessages } from './webview/messages' import { PlotsData } from './data' import { ErrorsModel } from './errors/model' +import { DecorationProvider } from './errors/decorationProvider' import { PlotsModel } from './model' import { collectEncodingElements, collectScale } from './paths/collect' import { PathsModel } from './paths/model' @@ -33,6 +34,9 @@ export class Plots extends BaseRepository { private readonly data: PlotsData private readonly errors: ErrorsModel + private readonly decorationProvider = this.dispose.track( + new DecorationProvider() + ) private webviewMessages: WebviewMessages @@ -52,7 +56,7 @@ export class Plots extends BaseRepository { new PlotsModel(this.dvcRoot, experiments, this.errors, workspaceState) ) this.paths = this.dispose.track( - new PathsModel(this.dvcRoot, workspaceState) + new PathsModel(this.dvcRoot, this.errors, workspaceState) ) this.webviewMessages = this.createWebviewMessageHandler( @@ -130,7 +134,11 @@ export class Plots extends BaseRepository { } private notifyChanged() { - this.paths.setSelectedRevisions(this.plots.getSelectedRevisions()) + const selectedRevisions = this.plots.getSelectedRevisions() + this.paths.setSelectedRevisions(selectedRevisions) + this.decorationProvider.setState( + this.errors.getErrorPaths(selectedRevisions) + ) this.pathsChanged.fire() void this.fetchMissingOrSendPlots() } diff --git a/extension/src/plots/paths/model.test.ts b/extension/src/plots/paths/model.test.ts index be40890be1..b1e4c6149d 100644 --- a/extension/src/plots/paths/model.test.ts +++ b/extension/src/plots/paths/model.test.ts @@ -5,6 +5,7 @@ import plotsDiffFixture from '../../test/fixtures/plotsDiff/output' import { buildMockMemento } from '../../test/util' import { PlotsType, TemplatePlotGroup } from '../webview/contract' import { EXPERIMENT_WORKSPACE_ID } from '../../cli/dvc/contract' +import { ErrorsModel } from '../errors/model' describe('PathsModel', () => { const mockDvcRoot = 'test' @@ -20,12 +21,19 @@ describe('PathsModel', () => { '1ba7bcd' ] + const buildMockErrorsModel = () => + ({ getPathErrors: () => undefined } as unknown as ErrorsModel) + it('should return the expected paths when given the default output fixture', () => { const comparisonType = new Set([PathType.COMPARISON]) const singleType = new Set([PathType.TEMPLATE_SINGLE]) const multiType = new Set([PathType.TEMPLATE_MULTI]) - const model = new PathsModel(mockDvcRoot, buildMockMemento()) + const model = new PathsModel( + mockDvcRoot, + buildMockErrorsModel(), + buildMockMemento() + ) model.transformAndSet(plotsDiffFixture, revisions, {}) model.setSelectedRevisions(revisions) expect(model.getTerminalNodes()).toStrictEqual([ @@ -132,7 +140,11 @@ describe('PathsModel', () => { const originalTemplateOrder = [originalSingleViewGroup, multiViewGroup] it('should retain the order of template paths when they are unselected', () => { - const model = new PathsModel(mockDvcRoot, buildMockMemento()) + const model = new PathsModel( + mockDvcRoot, + buildMockErrorsModel(), + buildMockMemento() + ) model.transformAndSet(plotsDiffFixture, revisions, {}) model.setSelectedRevisions([EXPERIMENT_WORKSPACE_ID]) @@ -150,7 +162,11 @@ describe('PathsModel', () => { }) it('should move unselected plots to the end when a reordering occurs', () => { - const model = new PathsModel(mockDvcRoot, buildMockMemento()) + const model = new PathsModel( + mockDvcRoot, + buildMockErrorsModel(), + buildMockMemento() + ) model.transformAndSet(plotsDiffFixture, revisions, {}) model.setSelectedRevisions([EXPERIMENT_WORKSPACE_ID]) @@ -176,7 +192,11 @@ describe('PathsModel', () => { }) it('should not update the order of plots when there are no revisions selected', () => { - const model = new PathsModel(mockDvcRoot, buildMockMemento()) + const model = new PathsModel( + mockDvcRoot, + buildMockErrorsModel(), + buildMockMemento() + ) model.transformAndSet(plotsDiffFixture, revisions, {}) model.setSelectedRevisions([EXPERIMENT_WORKSPACE_ID]) @@ -204,7 +224,11 @@ describe('PathsModel', () => { }) it('should not move plots which do not have the selected revisions if no reordering occurs', () => { - const model = new PathsModel(mockDvcRoot, buildMockMemento()) + const model = new PathsModel( + mockDvcRoot, + buildMockErrorsModel(), + buildMockMemento() + ) model.transformAndSet( { data: { ...plotsDiffFixture.data, ...previousPlotFixture.data } }, @@ -243,7 +267,11 @@ describe('PathsModel', () => { }) it('should move plots which do not have selected revisions to the end when a reordering occurs', () => { - const model = new PathsModel(mockDvcRoot, buildMockMemento()) + const model = new PathsModel( + mockDvcRoot, + buildMockErrorsModel(), + buildMockMemento() + ) model.transformAndSet( { data: { ...plotsDiffFixture.data, ...previousPlotFixture.data } }, [...revisions, commitBeforePlots], @@ -268,7 +296,11 @@ describe('PathsModel', () => { }) it('should move newly collected plot paths to the end', () => { - const model = new PathsModel(mockDvcRoot, buildMockMemento()) + const model = new PathsModel( + mockDvcRoot, + buildMockErrorsModel(), + buildMockMemento() + ) model.transformAndSet(plotsDiffFixture, revisions, {}) model.setSelectedRevisions([EXPERIMENT_WORKSPACE_ID]) @@ -285,7 +317,11 @@ describe('PathsModel', () => { }) it('should merge template plots groups when a path is unselected', () => { - const model = new PathsModel(mockDvcRoot, buildMockMemento()) + const model = new PathsModel( + mockDvcRoot, + buildMockErrorsModel(), + buildMockMemento() + ) model.transformAndSet(plotsDiffFixture, revisions, {}) model.setSelectedRevisions([EXPERIMENT_WORKSPACE_ID]) @@ -297,7 +333,11 @@ describe('PathsModel', () => { }) it('should retain the order of the comparison paths when changed', () => { - const model = new PathsModel(mockDvcRoot, buildMockMemento()) + const model = new PathsModel( + mockDvcRoot, + buildMockErrorsModel(), + buildMockMemento() + ) model.transformAndSet(plotsDiffFixture, revisions, {}) model.setSelectedRevisions([EXPERIMENT_WORKSPACE_ID]) @@ -319,7 +359,11 @@ describe('PathsModel', () => { }) it('should return the expected children from the test fixture', () => { - const model = new PathsModel(mockDvcRoot, buildMockMemento()) + const model = new PathsModel( + mockDvcRoot, + buildMockErrorsModel(), + buildMockMemento() + ) model.transformAndSet(plotsDiffFixture, revisions, {}) model.setSelectedRevisions([EXPERIMENT_WORKSPACE_ID]) @@ -336,7 +380,8 @@ describe('PathsModel', () => { parentPath: undefined, path: 'plots', revisions: new Set(revisions), - status: 2 + status: 2, + tooltip: undefined }, { descendantStatuses: [2, 2], @@ -344,7 +389,8 @@ describe('PathsModel', () => { parentPath: undefined, path: 'logs', revisions: new Set(revisions), - status: 2 + status: 2, + tooltip: undefined }, { descendantStatuses: [], @@ -353,6 +399,7 @@ describe('PathsModel', () => { path: 'predictions.json', revisions: new Set(revisions), status: 2, + tooltip: undefined, type: new Set([PathType.TEMPLATE_MULTI]) } ]) @@ -366,6 +413,7 @@ describe('PathsModel', () => { path: logsLoss, revisions: new Set(revisions), status: 2, + tooltip: undefined, type: new Set([PathType.TEMPLATE_SINGLE]) }, { @@ -375,6 +423,7 @@ describe('PathsModel', () => { path: logsAcc, revisions: new Set(revisions), status: 2, + tooltip: undefined, type: new Set([PathType.TEMPLATE_SINGLE]) } ]) @@ -395,6 +444,7 @@ describe('PathsModel', () => { path: logsLoss, revisions: new Set(revisions), status: 2, + tooltip: undefined, type: new Set([PathType.TEMPLATE_SINGLE]) }, { @@ -404,6 +454,7 @@ describe('PathsModel', () => { path: logsAcc, revisions: new Set(revisions), status: 2, + tooltip: undefined, type: new Set([PathType.TEMPLATE_SINGLE]) } ]) @@ -413,7 +464,11 @@ describe('PathsModel', () => { }) it('should not provide error as a path when the CLI throws an error', () => { - const model = new PathsModel(mockDvcRoot, buildMockMemento()) + const model = new PathsModel( + mockDvcRoot, + buildMockErrorsModel(), + buildMockMemento() + ) model.transformAndSet( { error: { diff --git a/extension/src/plots/paths/model.ts b/extension/src/plots/paths/model.ts index 74f75db7b3..39d1576b0e 100644 --- a/extension/src/plots/paths/model.ts +++ b/extension/src/plots/paths/model.ts @@ -15,16 +15,22 @@ import { import { MultiSourceEncoding } from '../multiSource/collect' import { isDvcError } from '../../cli/dvc/reader' import { PlotsOutputOrError } from '../../cli/dvc/contract' +import { getErrorTooltip } from '../../tree' +import { ErrorsModel } from '../errors/model' export class PathsModel extends PathSelectionModel { + private readonly errors: ErrorsModel + private templateOrder: TemplateOrder private comparisonPathsOrder: string[] private selectedRevisions: string[] = [] - constructor(dvcRoot: string, workspaceState: Memento) { + constructor(dvcRoot: string, errors: ErrorsModel, workspaceState: Memento) { super(dvcRoot, workspaceState, PersistenceKey.PLOT_PATH_STATUS) + this.errors = errors + this.templateOrder = this.revive(PersistenceKey.PLOT_TEMPLATE_ORDER, []) this.comparisonPathsOrder = this.revive( PersistenceKey.PLOT_COMPARISON_PATHS_ORDER, @@ -77,7 +83,8 @@ export class PathsModel extends PathSelectionModel { ...element, descendantStatuses: this.getTerminalNodeStatuses(element.path), hasChildren: this.getHasChildren(element, multiSourceEncoding), - status: this.status[element.path] + status: this.status[element.path], + tooltip: this.getTooltip(element.path) })) } @@ -177,4 +184,9 @@ export class PathsModel extends PathSelectionModel { private hasRevisions({ revisions }: PlotPath) { return this.selectedRevisions.some(revision => revisions.has(revision)) } + + private getTooltip(path: string) { + const error = this.errors.getPathErrors(path, this.selectedRevisions) + return error ? getErrorTooltip(error) : undefined + } } diff --git a/extension/src/plots/paths/tree.test.ts b/extension/src/plots/paths/tree.test.ts index 491329ef8f..085305b36b 100644 --- a/extension/src/plots/paths/tree.test.ts +++ b/extension/src/plots/paths/tree.test.ts @@ -70,7 +70,8 @@ describe('PlotsPathsTree', () => { join(__filename, 'resources', 'plots', 'stroke-dash-1-0.svg') ), label: 'A', - path: 'A' + path: 'A', + tooltip: undefined }, { collapsibleState: 0, @@ -80,7 +81,8 @@ describe('PlotsPathsTree', () => { join(__filename, 'resources', 'plots', 'circle.svg') ), label: 'Y', - path: 'Y' + path: 'Y', + tooltip: undefined } ]) }) diff --git a/extension/src/plots/paths/tree.ts b/extension/src/plots/paths/tree.ts index 3e321adc4c..8b415f0024 100644 --- a/extension/src/plots/paths/tree.ts +++ b/extension/src/plots/paths/tree.ts @@ -62,7 +62,8 @@ export class PlotsPathsTree extends BasePathSelectionTree { ? this.resourceLocator.getPlotsStrokeDashResource(value) : this.resourceLocator.getPlotsShapeResource(value), label, - path: label + path: label, + tooltip: undefined } }