From 0f6d5bf51af78d1851ea6e9f6178cec96dbea4ac Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Fri, 23 Sep 2022 16:02:14 +1000 Subject: [PATCH 01/11] rough prototype --- extension/src/experiments/columns/model.ts | 21 +++++-- extension/src/experiments/columns/tree.ts | 5 +- extension/src/path/selection/model.ts | 15 +---- extension/src/path/selection/tree.ts | 68 ++++++++++------------ extension/src/plots/index.ts | 12 +++- extension/src/plots/model/index.ts | 4 ++ extension/src/plots/paths/collect.ts | 35 +++++++++++ extension/src/plots/paths/model.ts | 31 ++++++++-- extension/src/plots/paths/tree.ts | 23 +++++++- extension/src/resourceLocator.ts | 13 ++++- 10 files changed, 162 insertions(+), 65 deletions(-) diff --git a/extension/src/experiments/columns/model.ts b/extension/src/experiments/columns/model.ts index 7d0c73ff36..f846a55632 100644 --- a/extension/src/experiments/columns/model.ts +++ b/extension/src/experiments/columns/model.ts @@ -63,7 +63,22 @@ export class ColumnsModel extends PathSelectionModel { ) } - public filterChildren(path?: string) { + public getChildren(path: string | undefined) { + return this.filterChildren(path).map(element => { + return { + ...element, + descendantStatuses: this.getTerminalNodeStatuses(element.path), + label: element.label, + status: this.status[element.path] + } + }) + } + + public hasNonDefaultColumns() { + return this.data.length > 1 + } + + private filterChildren(path?: string) { return this.data.filter(element => path ? element.parentPath === path @@ -73,10 +88,6 @@ export class ColumnsModel extends PathSelectionModel { ) } - public hasNonDefaultColumns() { - return this.data.length > 1 - } - private async transformAndSetColumns(data: ExperimentsOutput) { const [columns, paramsFiles] = await Promise.all([ collectColumns(data), diff --git a/extension/src/experiments/columns/tree.ts b/extension/src/experiments/columns/tree.ts index 58d7d10dca..df4f647982 100644 --- a/extension/src/experiments/columns/tree.ts +++ b/extension/src/experiments/columns/tree.ts @@ -31,7 +31,10 @@ export class ExperimentsColumnsTree extends BasePathSelectionTree this.transformElement({ ...element, dvcRoot })) } public getRepositoryStatuses(dvcRoot: string) { diff --git a/extension/src/path/selection/model.ts b/extension/src/path/selection/model.ts index ff486cbd04..1d577dd352 100644 --- a/extension/src/path/selection/model.ts +++ b/extension/src/path/selection/model.ts @@ -44,17 +44,6 @@ export abstract class PathSelectionModel< .map(element => ({ ...element, selected: !!this.status[element.path] })) } - public getChildren(path: string | undefined) { - return this.filterChildren(path).map(element => { - return { - ...element, - descendantStatuses: this.getTerminalNodeStatuses(element.path), - label: element.label, - status: this.status[element.path] - } - }) - } - public toggleStatus(path: string) { const status = this.getNextStatus(path) this.status[path] = status @@ -153,5 +142,7 @@ export abstract class PathSelectionModel< return this.persist(this.statusKey, this.status) } - abstract filterChildren(path?: string): T[] + abstract getChildren( + ...args: unknown[] + ): (T & { descendantStatuses: Status[]; label: string; status: Status })[] } diff --git a/extension/src/path/selection/tree.ts b/extension/src/path/selection/tree.ts index d7eed7ad51..90035ff1a4 100644 --- a/extension/src/path/selection/tree.ts +++ b/extension/src/path/selection/tree.ts @@ -23,7 +23,7 @@ export type PathSelectionItem = { collapsibleState: TreeItemCollapsibleState label: string path: string - iconPath: Resource + iconPath: Resource | Uri } export abstract class BasePathSelectionTree< @@ -35,8 +35,7 @@ export abstract class BasePathSelectionTree< public readonly onDidChangeTreeData: Event protected readonly workspace: T - - private readonly resourceLocator: ResourceLocator + protected readonly resourceLocator: ResourceLocator private readonly view: TreeView @@ -127,6 +126,33 @@ export abstract class BasePathSelectionTree< }${separator}${statuses.length}` } + protected transformElement(element: { + dvcRoot: string + descendantStatuses: Status[] + hasChildren: boolean + path: string + status: Status + label: string + }) { + const { dvcRoot, descendantStatuses, hasChildren, path, status, label } = + element + + const description = this.getDescription(descendantStatuses, '/') + const iconPath = this.getIconPath(status) + const collapsibleState = hasChildren + ? TreeItemCollapsibleState.Collapsed + : TreeItemCollapsibleState.None + + return { + collapsibleState, + description, + dvcRoot, + iconPath, + label, + path + } as PathSelectionItem + } + private updateDescriptionOnChange() { this.dispose.track( this.onDidChangeTreeData(() => { @@ -165,52 +191,22 @@ export abstract class BasePathSelectionTree< } if (this.isRoot(element)) { - return this.transformRepositoryChildren(element, undefined) + return this.getRepositoryChildren(element, undefined) } const { dvcRoot, path } = element - return this.transformRepositoryChildren(dvcRoot, path) + return this.getRepositoryChildren(dvcRoot, path) } private isRoot(element: string | PathSelectionItem): element is string { return typeof element === 'string' } - private transformRepositoryChildren( - dvcRoot: string, - path: string | undefined - ) { - return this.getRepositoryChildren(dvcRoot, path).map(element => { - const { descendantStatuses, hasChildren, path, status, label } = element - - const description = this.getDescription(descendantStatuses, '/') - const iconPath = this.getIconPath(status) - const collapsibleState = hasChildren - ? TreeItemCollapsibleState.Collapsed - : TreeItemCollapsibleState.None - - return { - collapsibleState, - description, - dvcRoot, - iconPath, - label, - path - } as PathSelectionItem - }) - } - abstract getRepositoryStatuses(dvcRoot: string): Status[] abstract getRepositoryChildren( dvcRoot: string, path: string | undefined - ): { - descendantStatuses: Status[] - hasChildren: boolean - label: string - path: string - status: Status - }[] + ): PathSelectionItem[] } diff --git a/extension/src/plots/index.ts b/extension/src/plots/index.ts index 5832b6d6c8..0d4c9346c1 100644 --- a/extension/src/plots/index.ts +++ b/extension/src/plots/index.ts @@ -4,7 +4,7 @@ import { PlotsData as TPlotsData } from './webview/contract' import { WebviewMessages } from './webview/messages' import { PlotsData } from './data' import { PlotsModel } from './model' -import { collectScale } from './paths/collect' +import { collectElementsFromEncoding, collectScale } from './paths/collect' import { PathsModel } from './paths/model' import { BaseWebview } from '../webview' import { ViewKey } from '../webview/constants' @@ -127,8 +127,14 @@ export class Plots extends BaseRepository { this.data.managedUpdate() } - public getChildPaths(path: string) { - return this.paths?.getChildren(path) || [] + public getChildPaths(path: string | undefined) { + const multiSourceEncoding = this.plots?.getMultiSourceData() || {} + + if (path && multiSourceEncoding[path]) { + return collectElementsFromEncoding(path, multiSourceEncoding) + } + + return this.paths?.getChildren(path, multiSourceEncoding) || [] } public getPathStatuses() { diff --git a/extension/src/plots/model/index.ts b/extension/src/plots/model/index.ts index 3e034519d4..7908fbf7ad 100644 --- a/extension/src/plots/model/index.ts +++ b/extension/src/plots/model/index.ts @@ -306,6 +306,10 @@ export class PlotsModel extends ModelWithPersistence { return this.sectionCollapsed } + public getMultiSourceData() { + return this.multiSourceEncoding + } + private removeStaleData() { return Promise.all([ this.removeStaleBranches(), diff --git a/extension/src/plots/paths/collect.ts b/extension/src/plots/paths/collect.ts index bb74c897bd..1eb47976ce 100644 --- a/extension/src/plots/paths/collect.ts +++ b/extension/src/plots/paths/collect.ts @@ -4,6 +4,8 @@ import { getParent, getPath, getPathArray } from '../../fileSystem/util' import { splitMatchedOrdered, definedAndNonEmpty } from '../../util/array' import { isMultiViewPlot } from '../vega/util' import { createTypedAccumulator } from '../../util/object' +import { StrokeDashValue } from '../multiSource/constants' +import { MultiSourceEncoding } from '../multiSource/collect' export enum PathType { COMPARISON = 'comparison', @@ -263,3 +265,36 @@ export const collectScale = (paths: PlotPath[] = []) => { } return acc } + +export type EncodingElement = { + type: string + value: StrokeDashValue + label: string +} + +export const isEncodingElement = ( + element: unknown +): element is EncodingElement => !!(element as EncodingElement)?.value + +export const collectElementsFromEncoding = ( + path: string, + multiSourceEncoding: MultiSourceEncoding +): EncodingElement[] => { + const encoding = multiSourceEncoding[path] + const { strokeDash } = encoding + const elements: { + type: string + value: StrokeDashValue + label: string + }[] = [] + const { domain, range } = strokeDash.scale + for (const [i, element] of domain.entries()) { + const child = { + label: element, + type: 'strokeDash', + value: range[i] + } + elements.push(child) + } + return elements +} diff --git a/extension/src/plots/paths/model.ts b/extension/src/plots/paths/model.ts index 5036ebb79a..034b2cb325 100644 --- a/extension/src/plots/paths/model.ts +++ b/extension/src/plots/paths/model.ts @@ -10,6 +10,7 @@ import { PlotsOutput } from '../../cli/dvc/reader' import { PathSelectionModel } from '../../path/selection/model' import { PersistenceKey } from '../../persistence/constants' import { performSimpleOrderedUpdate } from '../../util/array' +import { MultiSourceEncoding } from '../multiSource/collect' export class PathsModel extends PathSelectionModel { private templateOrder: TemplateOrder @@ -50,12 +51,23 @@ export class PathsModel extends PathSelectionModel { this.persist(PersistenceKey.PLOT_TEMPLATE_ORDER, this.templateOrder) } - public filterChildren(path: string | undefined): PlotPath[] { - return this.data.filter(element => { - if (!path) { - return !element.parentPath + public getChildren( + path: string | undefined, + multiSourceEncoding: MultiSourceEncoding = {} + ) { + return this.filterChildren(path).map(element => { + const hasChildren = + element.hasChildren === false + ? !!multiSourceEncoding[element.path] + : element.hasChildren + + return { + ...element, + descendantStatuses: this.getTerminalNodeStatuses(element.path), + hasChildren, + label: element.label, + status: this.status[element.path] } - return element.parentPath === path }) } @@ -95,4 +107,13 @@ export class PathsModel extends PathSelectionModel { .filter(plotPath => filter(type, plotPath)) .map(({ path }) => path) } + + private filterChildren(path: string | undefined): PlotPath[] { + return this.data.filter(element => { + if (!path) { + return !element.parentPath + } + return element.parentPath === path + }) + } } diff --git a/extension/src/plots/paths/tree.ts b/extension/src/plots/paths/tree.ts index 285b25d3e8..25c1f90aaa 100644 --- a/extension/src/plots/paths/tree.ts +++ b/extension/src/plots/paths/tree.ts @@ -1,3 +1,5 @@ +import { TreeItemCollapsibleState } from 'vscode' +import { isEncodingElement } from './collect' import { BasePathSelectionTree, PathSelectionItem @@ -30,8 +32,25 @@ export class PlotsPathsTree extends BasePathSelectionTree { ) } - public getRepositoryChildren(dvcRoot: string, path: string) { - return this.workspace.getRepository(dvcRoot).getChildPaths(path) + public getRepositoryChildren(dvcRoot: string, path: string | undefined) { + return this.workspace + .getRepository(dvcRoot) + .getChildPaths(path) + .map(element => { + if (isEncodingElement(element)) { + const { label, value } = element + return { + collapsibleState: TreeItemCollapsibleState.None, + description: undefined, + dvcRoot, + iconPath: this.resourceLocator.getPlotsStrokeDashResource(value), + label, + path: label + } + } + + return this.transformElement({ ...element, dvcRoot }) + }) } public getRepositoryStatuses(dvcRoot: string) { diff --git a/extension/src/resourceLocator.ts b/extension/src/resourceLocator.ts index 4383ca9b50..2aa07e56d5 100644 --- a/extension/src/resourceLocator.ts +++ b/extension/src/resourceLocator.ts @@ -1,12 +1,14 @@ import { Uri } from 'vscode' import { Disposable } from './class/dispose' +import { StrokeDashValue } from './plots/multiSource/constants' export type Resource = { dark: Uri; light: Uri } export enum IconName { CIRCLE_FILLED = 'circle-filled', CIRCLE_OUTLINE = 'circle-outline', - LOADING_SPIN = 'loading-spin' + LOADING_SPIN = 'loading-spin', + STROKE_DASH = 'stroke-dash' } export class ResourceLocator extends Disposable { @@ -45,6 +47,15 @@ export class ResourceLocator extends Disposable { ) } + public getPlotsStrokeDashResource(strokeDash: StrokeDashValue): Uri { + return Uri.joinPath( + this.extensionUri, + 'resources', + 'plots', + `stroke-dash-${strokeDash.join('-')}.svg` + ) + } + private getResourceLocations(...path: string[]): { dark: Uri; light: Uri } { return { dark: this.getResourceLocation('dark', ...path), From 19402f1d071d3dc5273fde050bd9893c1a2f21ee Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Fri, 23 Sep 2022 16:14:37 +1000 Subject: [PATCH 02/11] add stroke dash svgs --- extension/resources/plots/stroke-dash-1-0.svg | 3 +++ extension/resources/plots/stroke-dash-1-1.svg | 3 +++ extension/resources/plots/stroke-dash-2-1.svg | 3 +++ extension/resources/plots/stroke-dash-4-2.svg | 3 +++ extension/resources/plots/stroke-dash-4-4.svg | 3 +++ extension/resources/plots/stroke-dash-8-4.svg | 3 +++ extension/resources/plots/stroke-dash-8-8.svg | 3 +++ extension/src/cli/dvc/constants.ts | 2 +- 8 files changed, 22 insertions(+), 1 deletion(-) create mode 100644 extension/resources/plots/stroke-dash-1-0.svg create mode 100644 extension/resources/plots/stroke-dash-1-1.svg create mode 100644 extension/resources/plots/stroke-dash-2-1.svg create mode 100644 extension/resources/plots/stroke-dash-4-2.svg create mode 100644 extension/resources/plots/stroke-dash-4-4.svg create mode 100644 extension/resources/plots/stroke-dash-8-4.svg create mode 100644 extension/resources/plots/stroke-dash-8-8.svg diff --git a/extension/resources/plots/stroke-dash-1-0.svg b/extension/resources/plots/stroke-dash-1-0.svg new file mode 100644 index 0000000000..42487dcb6c --- /dev/null +++ b/extension/resources/plots/stroke-dash-1-0.svg @@ -0,0 +1,3 @@ + + + \ No newline at end of file diff --git a/extension/resources/plots/stroke-dash-1-1.svg b/extension/resources/plots/stroke-dash-1-1.svg new file mode 100644 index 0000000000..2c9b402063 --- /dev/null +++ b/extension/resources/plots/stroke-dash-1-1.svg @@ -0,0 +1,3 @@ + + + \ No newline at end of file diff --git a/extension/resources/plots/stroke-dash-2-1.svg b/extension/resources/plots/stroke-dash-2-1.svg new file mode 100644 index 0000000000..1b0abe5f10 --- /dev/null +++ b/extension/resources/plots/stroke-dash-2-1.svg @@ -0,0 +1,3 @@ + + + \ No newline at end of file diff --git a/extension/resources/plots/stroke-dash-4-2.svg b/extension/resources/plots/stroke-dash-4-2.svg new file mode 100644 index 0000000000..53f49167a0 --- /dev/null +++ b/extension/resources/plots/stroke-dash-4-2.svg @@ -0,0 +1,3 @@ + + + \ No newline at end of file diff --git a/extension/resources/plots/stroke-dash-4-4.svg b/extension/resources/plots/stroke-dash-4-4.svg new file mode 100644 index 0000000000..e65325a21f --- /dev/null +++ b/extension/resources/plots/stroke-dash-4-4.svg @@ -0,0 +1,3 @@ + + + \ No newline at end of file diff --git a/extension/resources/plots/stroke-dash-8-4.svg b/extension/resources/plots/stroke-dash-8-4.svg new file mode 100644 index 0000000000..12c1f51778 --- /dev/null +++ b/extension/resources/plots/stroke-dash-8-4.svg @@ -0,0 +1,3 @@ + + + \ No newline at end of file diff --git a/extension/resources/plots/stroke-dash-8-8.svg b/extension/resources/plots/stroke-dash-8-8.svg new file mode 100644 index 0000000000..4351b303fc --- /dev/null +++ b/extension/resources/plots/stroke-dash-8-8.svg @@ -0,0 +1,3 @@ + + + \ No newline at end of file diff --git a/extension/src/cli/dvc/constants.ts b/extension/src/cli/dvc/constants.ts index 29f554eac3..b56272240b 100644 --- a/extension/src/cli/dvc/constants.ts +++ b/extension/src/cli/dvc/constants.ts @@ -1,4 +1,4 @@ -export const MIN_CLI_VERSION = '2.24.0' +export const MIN_CLI_VERSION = '2.0.0' export const LATEST_TESTED_CLI_VERSION = '2.27.2' export const MAX_CLI_VERSION = '3' From 6b143a9c72a19b3a218bb9bf25622df6f30f578f Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Mon, 26 Sep 2022 15:06:38 +1000 Subject: [PATCH 03/11] add shape svgs --- extension/resources/plots/circle.svg | 3 +++ extension/resources/plots/diamond.svg | 3 +++ extension/resources/plots/square.svg | 3 +++ extension/resources/plots/triangle.svg | 3 +++ extension/src/plots/multiSource/constants.ts | 8 +------- 5 files changed, 13 insertions(+), 7 deletions(-) create mode 100644 extension/resources/plots/circle.svg create mode 100644 extension/resources/plots/diamond.svg create mode 100644 extension/resources/plots/square.svg create mode 100644 extension/resources/plots/triangle.svg diff --git a/extension/resources/plots/circle.svg b/extension/resources/plots/circle.svg new file mode 100644 index 0000000000..e8edbba837 --- /dev/null +++ b/extension/resources/plots/circle.svg @@ -0,0 +1,3 @@ + + + \ No newline at end of file diff --git a/extension/resources/plots/diamond.svg b/extension/resources/plots/diamond.svg new file mode 100644 index 0000000000..8645e370aa --- /dev/null +++ b/extension/resources/plots/diamond.svg @@ -0,0 +1,3 @@ + + + diff --git a/extension/resources/plots/square.svg b/extension/resources/plots/square.svg new file mode 100644 index 0000000000..691748af7d --- /dev/null +++ b/extension/resources/plots/square.svg @@ -0,0 +1,3 @@ + + + \ No newline at end of file diff --git a/extension/resources/plots/triangle.svg b/extension/resources/plots/triangle.svg new file mode 100644 index 0000000000..98aa65bab4 --- /dev/null +++ b/extension/resources/plots/triangle.svg @@ -0,0 +1,3 @@ + + + \ No newline at end of file diff --git a/extension/src/plots/multiSource/constants.ts b/extension/src/plots/multiSource/constants.ts index 725a811cd9..5a956f6336 100644 --- a/extension/src/plots/multiSource/constants.ts +++ b/extension/src/plots/multiSource/constants.ts @@ -9,13 +9,7 @@ export const StrokeDash = [ ] as const export type StrokeDashValue = typeof StrokeDash[number] -export const Shape = [ - 'square', - 'circle', - 'triangle', - 'diamond', - 'cross' -] as const +export const Shape = ['square', 'circle', 'triangle', 'diamond'] as const export type ShapeValue = typeof Shape[number] export type Scale = { From 782487806df40974557ef4c9b996d721ec3b3c13 Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Mon, 26 Sep 2022 15:17:18 +1000 Subject: [PATCH 04/11] add shape to collection --- extension/src/plots/paths/collect.ts | 44 +++++++++++++++++++++------- extension/src/plots/paths/tree.ts | 9 ++++-- extension/src/resourceLocator.ts | 6 +++- 3 files changed, 44 insertions(+), 15 deletions(-) diff --git a/extension/src/plots/paths/collect.ts b/extension/src/plots/paths/collect.ts index 1eb47976ce..31400ca673 100644 --- a/extension/src/plots/paths/collect.ts +++ b/extension/src/plots/paths/collect.ts @@ -4,7 +4,7 @@ import { getParent, getPath, getPathArray } from '../../fileSystem/util' import { splitMatchedOrdered, definedAndNonEmpty } from '../../util/array' import { isMultiViewPlot } from '../vega/util' import { createTypedAccumulator } from '../../util/object' -import { StrokeDashValue } from '../multiSource/constants' +import { ShapeValue, StrokeDashValue } from '../multiSource/constants' import { MultiSourceEncoding } from '../multiSource/collect' export enum PathType { @@ -266,12 +266,23 @@ export const collectScale = (paths: PlotPath[] = []) => { return acc } -export type EncodingElement = { - type: string - value: StrokeDashValue - label: string +export enum EncodingType { + SHAPE = 'shape', + STROKE_DASH = 'strokeDash' } +export type EncodingElement = + | { + type: EncodingType.STROKE_DASH + value: StrokeDashValue + label: string + } + | { + type: EncodingType.SHAPE + value: ShapeValue + label: string + } + export const isEncodingElement = ( element: unknown ): element is EncodingElement => !!(element as EncodingElement)?.value @@ -282,19 +293,30 @@ export const collectElementsFromEncoding = ( ): EncodingElement[] => { const encoding = multiSourceEncoding[path] const { strokeDash } = encoding - const elements: { - type: string - value: StrokeDashValue - label: string - }[] = [] + const elements: EncodingElement[] = [] const { domain, range } = strokeDash.scale for (const [i, element] of domain.entries()) { const child = { label: element, - type: 'strokeDash', + type: EncodingType.STROKE_DASH as EncodingType.STROKE_DASH, value: range[i] } elements.push(child) } + + const { shape } = encoding + + if (shape) { + const { domain, range } = shape.scale + for (const [i, element] of domain.entries()) { + const child = { + label: element, + type: EncodingType.SHAPE as EncodingType.SHAPE, + value: range[i] + } + elements.push(child) + } + } + return elements } diff --git a/extension/src/plots/paths/tree.ts b/extension/src/plots/paths/tree.ts index 25c1f90aaa..fe41a7cdc6 100644 --- a/extension/src/plots/paths/tree.ts +++ b/extension/src/plots/paths/tree.ts @@ -1,5 +1,5 @@ import { TreeItemCollapsibleState } from 'vscode' -import { isEncodingElement } from './collect' +import { EncodingType, isEncodingElement } from './collect' import { BasePathSelectionTree, PathSelectionItem @@ -38,12 +38,15 @@ export class PlotsPathsTree extends BasePathSelectionTree { .getChildPaths(path) .map(element => { if (isEncodingElement(element)) { - const { label, value } = element + const { label, type, value } = element return { collapsibleState: TreeItemCollapsibleState.None, description: undefined, dvcRoot, - iconPath: this.resourceLocator.getPlotsStrokeDashResource(value), + iconPath: + type === EncodingType.STROKE_DASH + ? this.resourceLocator.getPlotsStrokeDashResource(value) + : this.resourceLocator.getPlotsShapeResource(value), label, path: label } diff --git a/extension/src/resourceLocator.ts b/extension/src/resourceLocator.ts index 2aa07e56d5..46c276ea4e 100644 --- a/extension/src/resourceLocator.ts +++ b/extension/src/resourceLocator.ts @@ -1,6 +1,6 @@ import { Uri } from 'vscode' import { Disposable } from './class/dispose' -import { StrokeDashValue } from './plots/multiSource/constants' +import { ShapeValue, StrokeDashValue } from './plots/multiSource/constants' export type Resource = { dark: Uri; light: Uri } @@ -56,6 +56,10 @@ export class ResourceLocator extends Disposable { ) } + public getPlotsShapeResource(shape: ShapeValue): Uri { + return Uri.joinPath(this.extensionUri, 'resources', 'plots', `${shape}.svg`) + } + private getResourceLocations(...path: string[]): { dark: Uri; light: Uri } { return { dark: this.getResourceLocation('dark', ...path), From 0490a515ffc5066be2d08f71c160b30f78838ef5 Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Mon, 26 Sep 2022 18:56:54 +1000 Subject: [PATCH 05/11] fix stroke dash legend entry when shape is used --- extension/src/plots/vega/util.ts | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/extension/src/plots/vega/util.ts b/extension/src/plots/vega/util.ts index c6aa688f23..3018cec357 100644 --- a/extension/src/plots/vega/util.ts +++ b/extension/src/plots/vega/util.ts @@ -153,7 +153,7 @@ export const getSpecEncodingUpdate = ({ ...shape, legend: { disable: true } } - encoding.detail = { field: shape.field } + encoding.detail = shape } return { @@ -292,8 +292,24 @@ export const reverseOfLegendSuppressionUpdate = () => ({ spec: { encoding: { color: { legend: { disable: false } }, - shape: { legend: { disable: false } }, - strokeDash: { legend: { disable: false } } + shape: { + legend: { + disable: false + } + }, + strokeDash: { + legend: { + disable: false, + encode: { + symbols: { + update: { + fill: { value: 'transparent' }, + stroke: { value: 'grey' } + } + } + } + } + } } } }) From 09479308d0bae73fe6df4043c843e7474bdd0f5e Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Mon, 26 Sep 2022 19:32:30 +1000 Subject: [PATCH 06/11] fix shape legend --- extension/src/plots/vega/util.test.ts | 7 +++++-- extension/src/plots/vega/util.ts | 25 ++++++++++++++----------- 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/extension/src/plots/vega/util.test.ts b/extension/src/plots/vega/util.test.ts index b4994f93c4..8179f71c78 100644 --- a/extension/src/plots/vega/util.test.ts +++ b/extension/src/plots/vega/util.test.ts @@ -282,14 +282,17 @@ describe('reverseOfLegendSuppressionUpdate', () => { shape: { field: 'shape-field', legend: { - disable: true + disable: true, + symbolFillColor: 'blue' }, scale: { domain: [], range: [] } }, strokeDash: { field: 'strokeDash-field', legend: { - disable: true + disable: true, + symbolFillColor: 'blue', + symbolStrokeColor: 'red' }, scale: { domain: [], range: [] } } diff --git a/extension/src/plots/vega/util.ts b/extension/src/plots/vega/util.ts index 3018cec357..8244d4f687 100644 --- a/extension/src/plots/vega/util.ts +++ b/extension/src/plots/vega/util.ts @@ -103,11 +103,14 @@ export type Encoding = { strokeDash?: StrokeDashEncoding & { legend: { disable: boolean + symbolFillColor: string + symbolStrokeColor: string } } shape?: ShapeEncoding & { legend: { disable: boolean + symbolFillColor: string } } detail?: { @@ -145,13 +148,21 @@ export const getSpecEncodingUpdate = ({ if (strokeDash) { encoding.strokeDash = { ...strokeDash, - legend: { disable: true } + legend: { + disable: true, + symbolFillColor: 'transparent', + symbolStrokeColor: 'grey' + } } } + if (shape) { encoding.shape = { ...shape, - legend: { disable: true } + legend: { + disable: true, + symbolFillColor: 'grey' + } } encoding.detail = shape } @@ -299,15 +310,7 @@ export const reverseOfLegendSuppressionUpdate = () => ({ }, strokeDash: { legend: { - disable: false, - encode: { - symbols: { - update: { - fill: { value: 'transparent' }, - stroke: { value: 'grey' } - } - } - } + disable: false } } } From abb38f7c2be4880f18399118f1b25f7754ecbadc Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Tue, 27 Sep 2022 10:14:59 +1000 Subject: [PATCH 07/11] centre triangle --- extension/resources/plots/triangle.svg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extension/resources/plots/triangle.svg b/extension/resources/plots/triangle.svg index 98aa65bab4..b8f8a4a5c9 100644 --- a/extension/resources/plots/triangle.svg +++ b/extension/resources/plots/triangle.svg @@ -1,3 +1,3 @@ - + \ No newline at end of file From e68f2f72a6706ae7e4bc63dba4f7f06f5ffbc4f6 Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Tue, 27 Sep 2022 10:37:49 +1000 Subject: [PATCH 08/11] revert min version as change is backward compatible --- extension/src/cli/dvc/constants.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extension/src/cli/dvc/constants.ts b/extension/src/cli/dvc/constants.ts index b56272240b..29f554eac3 100644 --- a/extension/src/cli/dvc/constants.ts +++ b/extension/src/cli/dvc/constants.ts @@ -1,4 +1,4 @@ -export const MIN_CLI_VERSION = '2.0.0' +export const MIN_CLI_VERSION = '2.24.0' export const LATEST_TESTED_CLI_VERSION = '2.27.2' export const MAX_CLI_VERSION = '3' From ab0223097f40dbc2933ca95ec8fc2554b3dde349 Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Tue, 27 Sep 2022 10:54:25 +1000 Subject: [PATCH 09/11] get new element collection under test --- extension/src/plots/index.ts | 4 +- extension/src/plots/paths/collect.test.ts | 60 ++++++++++++++++++++++- extension/src/plots/paths/collect.ts | 7 ++- 3 files changed, 67 insertions(+), 4 deletions(-) diff --git a/extension/src/plots/index.ts b/extension/src/plots/index.ts index 0d4c9346c1..a31b5fdcd3 100644 --- a/extension/src/plots/index.ts +++ b/extension/src/plots/index.ts @@ -4,7 +4,7 @@ import { PlotsData as TPlotsData } from './webview/contract' import { WebviewMessages } from './webview/messages' import { PlotsData } from './data' import { PlotsModel } from './model' -import { collectElementsFromEncoding, collectScale } from './paths/collect' +import { collectEncodingElements, collectScale } from './paths/collect' import { PathsModel } from './paths/model' import { BaseWebview } from '../webview' import { ViewKey } from '../webview/constants' @@ -131,7 +131,7 @@ export class Plots extends BaseRepository { const multiSourceEncoding = this.plots?.getMultiSourceData() || {} if (path && multiSourceEncoding[path]) { - return collectElementsFromEncoding(path, multiSourceEncoding) + return collectEncodingElements(path, multiSourceEncoding) } return this.paths?.getChildren(path, multiSourceEncoding) || [] diff --git a/extension/src/plots/paths/collect.test.ts b/extension/src/plots/paths/collect.test.ts index 0e3f0a197d..d5347fadb0 100644 --- a/extension/src/plots/paths/collect.test.ts +++ b/extension/src/plots/paths/collect.test.ts @@ -1,8 +1,14 @@ import { join } from 'path' import { VisualizationSpec } from 'react-vega' -import { collectPaths, collectTemplateOrder } from './collect' +import { + collectEncodingElements, + collectPaths, + collectTemplateOrder, + EncodingType +} from './collect' import { TemplatePlotGroup, PlotsType } from '../webview/contract' import plotsDiffFixture from '../../test/fixtures/plotsDiff/output' +import { Shape, StrokeDash } from '../multiSource/constants' describe('collectPath', () => { it('should return the expected data from the test fixture', () => { @@ -380,3 +386,55 @@ describe('collectTemplateOrder', () => { ]) }) }) + +describe('collectEncodingElements', () => { + it('should return an empty array if there is no multi source encoding for a path', () => { + const elements = collectEncodingElements(__filename, {}) + expect(elements).toStrictEqual([]) + }) + + it('should collect encoding elements from multi source encoding', () => { + const elements = collectEncodingElements(__filename, { + [__filename]: { + shape: { + field: 'filename', + scale: { domain: ['X', 'Y'], range: [Shape[0], Shape[1]] } + }, + strokeDash: { + field: 'field', + scale: { + domain: ['A', 'B', 'C'], + range: [StrokeDash[0], StrokeDash[1], StrokeDash[2]] + } + } + } + }) + expect(elements).toStrictEqual([ + { + label: 'A', + type: EncodingType.STROKE_DASH, + value: StrokeDash[0] + }, + { + label: 'B', + type: EncodingType.STROKE_DASH, + value: StrokeDash[1] + }, + { + label: 'C', + type: EncodingType.STROKE_DASH, + value: StrokeDash[2] + }, + { + label: 'X', + type: EncodingType.SHAPE, + value: Shape[0] + }, + { + label: 'Y', + type: EncodingType.SHAPE, + value: Shape[1] + } + ]) + }) +}) diff --git a/extension/src/plots/paths/collect.ts b/extension/src/plots/paths/collect.ts index 31400ca673..83c94f6763 100644 --- a/extension/src/plots/paths/collect.ts +++ b/extension/src/plots/paths/collect.ts @@ -287,11 +287,16 @@ export const isEncodingElement = ( element: unknown ): element is EncodingElement => !!(element as EncodingElement)?.value -export const collectElementsFromEncoding = ( +export const collectEncodingElements = ( path: string, multiSourceEncoding: MultiSourceEncoding ): EncodingElement[] => { const encoding = multiSourceEncoding[path] + + if (!encoding) { + return [] + } + const { strokeDash } = encoding const elements: EncodingElement[] = [] const { domain, range } = strokeDash.scale From c4d23b6f79c2529d80d70958f3fc13cf904963ca Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Tue, 27 Sep 2022 11:04:36 +1000 Subject: [PATCH 10/11] refactor collection --- extension/src/plots/paths/collect.ts | 47 +++++++++++++++------------- 1 file changed, 26 insertions(+), 21 deletions(-) diff --git a/extension/src/plots/paths/collect.ts b/extension/src/plots/paths/collect.ts index 83c94f6763..39564ec95b 100644 --- a/extension/src/plots/paths/collect.ts +++ b/extension/src/plots/paths/collect.ts @@ -4,7 +4,12 @@ import { getParent, getPath, getPathArray } from '../../fileSystem/util' import { splitMatchedOrdered, definedAndNonEmpty } from '../../util/array' import { isMultiViewPlot } from '../vega/util' import { createTypedAccumulator } from '../../util/object' -import { ShapeValue, StrokeDashValue } from '../multiSource/constants' +import { + ShapeScale, + ShapeValue, + StrokeDashScale, + StrokeDashValue +} from '../multiSource/constants' import { MultiSourceEncoding } from '../multiSource/collect' export enum PathType { @@ -287,6 +292,22 @@ export const isEncodingElement = ( element: unknown ): element is EncodingElement => !!(element as EncodingElement)?.value +const collectElements = ( + acc: EncodingElement[], + scale: StrokeDashScale | ShapeScale, + type: EncodingType +): void => { + const { domain, range } = scale + for (const [i, element] of domain.entries()) { + const child = { + label: element, + type, + value: range[i] + } + acc.push(child as EncodingElement) + } +} + export const collectEncodingElements = ( path: string, multiSourceEncoding: MultiSourceEncoding @@ -298,30 +319,14 @@ export const collectEncodingElements = ( } const { strokeDash } = encoding - const elements: EncodingElement[] = [] - const { domain, range } = strokeDash.scale - for (const [i, element] of domain.entries()) { - const child = { - label: element, - type: EncodingType.STROKE_DASH as EncodingType.STROKE_DASH, - value: range[i] - } - elements.push(child) - } + const acc: EncodingElement[] = [] + collectElements(acc, strokeDash.scale, EncodingType.STROKE_DASH) const { shape } = encoding if (shape) { - const { domain, range } = shape.scale - for (const [i, element] of domain.entries()) { - const child = { - label: element, - type: EncodingType.SHAPE as EncodingType.SHAPE, - value: range[i] - } - elements.push(child) - } + collectElements(acc, shape.scale, EncodingType.SHAPE) } - return elements + return acc } From e10c8d60347c30ec5b51464b4963105160d68fdc Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Tue, 27 Sep 2022 15:17:21 +1000 Subject: [PATCH 11/11] test creation of children from encoding --- extension/src/plots/paths/tree.test.ts | 83 ++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 extension/src/plots/paths/tree.test.ts diff --git a/extension/src/plots/paths/tree.test.ts b/extension/src/plots/paths/tree.test.ts new file mode 100644 index 0000000000..cee85a8e86 --- /dev/null +++ b/extension/src/plots/paths/tree.test.ts @@ -0,0 +1,83 @@ +import { Uri } from 'vscode' +import { Disposable, Disposer } from '@hediet/std/disposable' +import { EncodingType } from './collect' +import { PlotsPathsTree } from './tree' +import { WorkspacePlots } from '../workspace' +import { ResourceLocator } from '../../resourceLocator' +import { InternalCommands } from '../../commands/internal' +import { Plots } from '..' +import { buildMockedEventEmitter } from '../../test/util/jest' +import { Shape, StrokeDash } from '../multiSource/constants' +import { join } from '../../test/util/path' + +const mockedDisposable = jest.mocked(Disposable) +const mockedGetChildPaths = jest.fn() + +jest.mock('vscode') +jest.mock('@hediet/std/disposable') + +beforeEach(() => { + jest.resetAllMocks() + mockedDisposable.fn.mockReturnValue({ + track: function (disposable: T): T { + return disposable + } + } as unknown as (() => void) & Disposer) +}) + +describe('PlotsPathsTree', () => { + it('should return the correct children for multi source plots (encoding elements)', () => { + const mockedWorkspacePlots = { + getRepository: () => + ({ getChildPaths: mockedGetChildPaths } as unknown as Plots), + pathsChanged: buildMockedEventEmitter() + } as unknown as WorkspacePlots + const mockedInternalCommands = { + registerExternalCommand: jest.fn() + } as unknown as InternalCommands + const resourceLocator = new ResourceLocator(Uri.file(__filename)) + + const plotsPathTree = new PlotsPathsTree( + mockedWorkspacePlots, + mockedInternalCommands, + resourceLocator + ) + + mockedGetChildPaths.mockReturnValueOnce([ + { + label: 'A', + type: EncodingType.STROKE_DASH, + value: StrokeDash[0] + }, + { + label: 'Y', + type: EncodingType.SHAPE, + value: Shape[1] + } + ]) + + const children = plotsPathTree.getRepositoryChildren(__dirname, undefined) + expect(children).toStrictEqual([ + { + collapsibleState: 0, + description: undefined, + dvcRoot: __dirname, + iconPath: Uri.file( + join(__filename, 'resources', 'plots', 'stroke-dash-1-0.svg') + ), + label: 'A', + path: 'A' + }, + { + collapsibleState: 0, + description: undefined, + dvcRoot: __dirname, + iconPath: Uri.file( + join(__filename, 'resources', 'plots', 'circle.svg') + ), + label: 'Y', + path: 'Y' + } + ]) + }) +})