Skip to content

Show plots errors in plots tree #3522

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.

Sign up for GitHub

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

Merged
mattseddon merged 1 commit into from
Apr 2, 2023
Merged

Show plots errors in plots tree #3522

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 60 additions & 30 deletions extension/src/experiments/columns/tree.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,23 +124,26 @@ describe('ExperimentsColumnsTree', () => {
dvcRoot: mockedDvcRoot,
iconPath: mockedSelectedCheckbox,
label: timestampColumn.label,
path: timestampColumn.path
path: timestampColumn.path,
tooltip: undefined
},
{
collapsibleState: 1,
description: undefined,
dvcRoot: mockedDvcRoot,
iconPath: mockedSelectedCheckbox,
label: 'summary.json',
path: buildMetricOrParamPath(ColumnType.METRICS, 'summary.json')
path: buildMetricOrParamPath(ColumnType.METRICS, 'summary.json'),
tooltip: undefined
},
{
collapsibleState: 1,
description: undefined,
dvcRoot: mockedDvcRoot,
iconPath: mockedSelectedCheckbox,
label: 'params.yaml',
path: buildMetricOrParamPath(ColumnType.PARAMS, 'params.yaml')
path: buildMetricOrParamPath(ColumnType.PARAMS, 'params.yaml'),
tooltip: undefined
},
{
collapsibleState: 1,
Expand All @@ -151,31 +154,35 @@ describe('ExperimentsColumnsTree', () => {
path: buildMetricOrParamPath(
ColumnType.PARAMS,
join('nested', 'params.yaml')
)
),
tooltip: undefined
},
{
collapsibleState: 1,
description: undefined,
dvcRoot: mockedDvcRoot,
iconPath: mockedSelectedCheckbox,
label: 'data',
path: buildDepPath('data')
path: buildDepPath('data'),
tooltip: undefined
},
{
collapsibleState: 1,
description: undefined,
dvcRoot: mockedDvcRoot,
iconPath: mockedSelectedCheckbox,
label: 'src',
path: buildDepPath('src')
path: buildDepPath('src'),
tooltip: undefined
},
{
collapsibleState: 0,
description: undefined,
dvcRoot: mockedDvcRoot,
iconPath: mockedSelectedCheckbox,
label: 'model.pkl',
path: buildDepPath('model.pkl')
path: buildDepPath('model.pkl'),
tooltip: undefined
}
])
})
Expand Down Expand Up @@ -211,23 +218,26 @@ describe('ExperimentsColumnsTree', () => {
dvcRoot: mockedDvcRoot,
iconPath: mockedSelectedCheckbox,
label: timestampColumn.label,
path: timestampColumn.path
path: timestampColumn.path,
tooltip: undefined
},
{
collapsibleState: 1,
description: undefined,
dvcRoot: mockedDvcRoot,
iconPath: mockedSelectedCheckbox,
label: 'summary.json',
path: buildMetricOrParamPath(ColumnType.METRICS, 'summary.json')
path: buildMetricOrParamPath(ColumnType.METRICS, 'summary.json'),
tooltip: undefined
},
{
collapsibleState: 1,
description: undefined,
dvcRoot: mockedDvcRoot,
iconPath: mockedSelectedCheckbox,
label: 'params.yaml',
path: paramsPath
path: paramsPath,
tooltip: undefined
},
{
collapsibleState: 1,
Expand All @@ -238,31 +248,35 @@ describe('ExperimentsColumnsTree', () => {
path: buildMetricOrParamPath(
ColumnType.PARAMS,
join('nested', 'params.yaml')
)
),
tooltip: undefined
},
{
collapsibleState: 1,
description: undefined,
dvcRoot: mockedDvcRoot,
iconPath: mockedSelectedCheckbox,
label: 'data',
path: buildDepPath('data')
path: buildDepPath('data'),
tooltip: undefined
},
{
collapsibleState: 1,
description: undefined,
dvcRoot: mockedDvcRoot,
iconPath: mockedSelectedCheckbox,
label: 'src',
path: buildDepPath('src')
path: buildDepPath('src'),
tooltip: undefined
},
{
collapsibleState: 0,
description: undefined,
dvcRoot: mockedDvcRoot,
iconPath: mockedSelectedCheckbox,
label: 'model.pkl',
path: buildDepPath('model.pkl')
path: buildDepPath('model.pkl'),
tooltip: undefined
}
])

Expand All @@ -276,15 +290,17 @@ describe('ExperimentsColumnsTree', () => {
descendantStatuses: [Status.UNSELECTED, Status.SELECTED],
hasChildren: true,
label: getLabel(param.path),
status: Status.INDETERMINATE
status: Status.INDETERMINATE,
tooltip: undefined
}
}
return {
...param,
descendantStatuses: undefined,
hasChildren: false,
label: getLabel(param.path),
status: Status.SELECTED
status: Status.SELECTED,
tooltip: undefined
}
})
)
Expand All @@ -295,7 +311,8 @@ describe('ExperimentsColumnsTree', () => {
dvcRoot: mockedDvcRoot,
iconPath: mockedSelectedCheckbox,
label: 'params.yaml',
path: paramsPath
path: paramsPath,
tooltip: undefined
})
expect(grandChildren).toStrictEqual([
{
Expand All @@ -304,55 +321,62 @@ describe('ExperimentsColumnsTree', () => {
dvcRoot: mockedDvcRoot,
iconPath: mockedSelectedCheckbox,
label: 'code_names',
path: appendColumnToPath(paramsPath, 'code_names')
path: appendColumnToPath(paramsPath, 'code_names'),
tooltip: undefined
},
{
collapsibleState: 0,
description: undefined,
dvcRoot: mockedDvcRoot,
iconPath: mockedSelectedCheckbox,
label: 'epochs',
path: appendColumnToPath(paramsPath, 'epochs')
path: appendColumnToPath(paramsPath, 'epochs'),
tooltip: undefined
},
{
collapsibleState: 0,
description: undefined,
dvcRoot: mockedDvcRoot,
iconPath: mockedSelectedCheckbox,
label: 'learning_rate',
path: appendColumnToPath(paramsPath, 'learning_rate')
path: appendColumnToPath(paramsPath, 'learning_rate'),
tooltip: undefined
},
{
collapsibleState: 0,
description: undefined,
dvcRoot: mockedDvcRoot,
iconPath: mockedSelectedCheckbox,
label: 'dvc_logs_dir',
path: appendColumnToPath(paramsPath, 'dvc_logs_dir')
path: appendColumnToPath(paramsPath, 'dvc_logs_dir'),
tooltip: undefined
},
{
collapsibleState: 0,
description: undefined,
dvcRoot: mockedDvcRoot,
iconPath: mockedSelectedCheckbox,
label: 'log_file',
path: appendColumnToPath(paramsPath, 'log_file')
path: appendColumnToPath(paramsPath, 'log_file'),
tooltip: undefined
},
{
collapsibleState: 0,
description: undefined,
dvcRoot: mockedDvcRoot,
iconPath: mockedSelectedCheckbox,
label: 'dropout',
path: appendColumnToPath(paramsPath, 'dropout')
path: appendColumnToPath(paramsPath, 'dropout'),
tooltip: undefined
},
{
collapsibleState: 1,
description: '1/2',
dvcRoot: mockedDvcRoot,
iconPath: mockedIndeterminateCheckbox,
label: 'process',
path: processPath
path: processPath,
tooltip: undefined
}
])

Expand All @@ -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({
Expand All @@ -373,7 +398,8 @@ describe('ExperimentsColumnsTree', () => {
dvcRoot: mockedDvcRoot,
iconPath: mockedIndeterminateCheckbox,
label: 'process',
path: processPath
path: processPath,
tooltip: undefined
})

expect(greatGrandChildren).toStrictEqual([
Expand All @@ -388,7 +414,8 @@ describe('ExperimentsColumnsTree', () => {
'params.yaml',
'process',
'threshold'
)
),
tooltip: undefined
},
{
collapsibleState: 0,
Expand All @@ -401,7 +428,8 @@ describe('ExperimentsColumnsTree', () => {
'params.yaml',
'process',
'test_arg'
)
),
tooltip: undefined
}
])
})
Expand Down Expand Up @@ -454,7 +482,8 @@ describe('ExperimentsColumnsTree', () => {
dvcRoot: mockedDvcRoot,
iconPath: mockedSelectedCheckbox,
label: filename,
path: relParamsPath
path: relParamsPath,
tooltip: undefined
}

const treeItem = experimentsColumnsTree.getTreeItem(columnsItem)
Expand Down Expand Up @@ -494,7 +523,8 @@ describe('ExperimentsColumnsTree', () => {
dvcRoot: mockedDvcRoot,
iconPath: mockedEmptyCheckbox,
label: filename,
path: relParamsPath
path: relParamsPath,
tooltip: undefined
}

const treeItem = experimentsColumnsTree.getTreeItem(columnsItem)
Expand Down
24 changes: 19 additions & 5 deletions extension/src/path/selection/tree.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {
Event,
MarkdownString,
TreeDataProvider,
TreeItem,
TreeItemCollapsibleState,
Expand All @@ -24,6 +25,7 @@ export type PathSelectionItem = {
label: string | undefined
path: string
iconPath: Resource | Uri
tooltip: MarkdownString | undefined
}

export abstract class BasePathSelectionTree<
Expand Down Expand Up @@ -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)

Expand All @@ -90,6 +92,9 @@ export abstract class BasePathSelectionTree<
if (description) {
treeItem.description = description
}
if (tooltip) {
treeItem.tooltip = tooltip
Copy link
Contributor

@sroy3 sroy3 Mar 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the point of checking for the element before assigning it? If tooltip is undefined, treeItem.tooltip is undefined as well anyway, no? I'm asking because I'm probably missing something.

Copy link
Contributor Author

@mattseddon mattseddon Mar 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are right, there is no point. I'll update

}

return treeItem
}
Expand Down Expand Up @@ -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)
Expand All @@ -148,8 +161,9 @@ export abstract class BasePathSelectionTree<
dvcRoot,
iconPath,
label,
path
} as PathSelectionItem
path,
tooltip
}
}

private updateDescriptionOnChange() {
Expand Down
Loading