From 55a3172d8e030b261daea05372b59294f256703c Mon Sep 17 00:00:00 2001 From: julieg18 Date: Mon, 2 Oct 2023 11:25:31 -0500 Subject: [PATCH 01/12] first iteration --- extension/src/fileSystem/index.test.ts | 78 ++++++----- extension/src/fileSystem/index.ts | 41 +++++- extension/src/fileSystem/util.ts | 2 + extension/src/pipeline/quickPick.test.ts | 125 +++++++++++------- extension/src/pipeline/quickPick.ts | 91 +++++++++---- .../src/test/suite/pipeline/index.test.ts | 5 +- extension/src/vscode/resourcePicker.ts | 24 +++- extension/src/vscode/title.ts | 2 + 8 files changed, 255 insertions(+), 113 deletions(-) diff --git a/extension/src/fileSystem/index.test.ts b/extension/src/fileSystem/index.test.ts index 25730afe61..0c39f28a9c 100644 --- a/extension/src/fileSystem/index.test.ts +++ b/extension/src/fileSystem/index.test.ts @@ -23,7 +23,7 @@ import { getPidFromFile, getEntryFromJsonFile, addPlotToDvcYamlFile, - loadDataFile + loadDataFiles } from '.' import { dvcDemoPath } from '../test/util' import { DOT_DVC } from '../cli/dvc/constants' @@ -63,17 +63,22 @@ beforeEach(() => { jest.resetAllMocks() }) -describe('loadDataFile', () => { +describe('loadDataFiles', () => { it('should load in csv file contents', async () => { const mockCsvContent = ['epoch,acc', '10,0.69', '11,0.345'].join('\n') mockedReadFileSync.mockReturnValueOnce(mockCsvContent) - const result = await loadDataFile('values.csv') + const result = await loadDataFiles(['values.csv']) expect(result).toStrictEqual([ - { acc: 0.69, epoch: 10 }, - { acc: 0.345, epoch: 11 } + { + data: [ + { acc: 0.69, epoch: 10 }, + { acc: 0.345, epoch: 11 } + ], + file: 'values.csv' + } ]) }) @@ -85,11 +90,16 @@ describe('loadDataFile', () => { mockedReadFileSync.mockReturnValueOnce(mockJsonContent) - const result = await loadDataFile('values.json') + const result = await loadDataFiles(['values.json']) expect(result).toStrictEqual([ - { acc: 0.69, epoch: 10 }, - { acc: 0.345, epoch: 11 } + { + data: [ + { acc: 0.69, epoch: 10 }, + { acc: 0.345, epoch: 11 } + ], + file: 'values.json' + } ]) }) @@ -98,11 +108,16 @@ describe('loadDataFile', () => { mockedReadFileSync.mockReturnValueOnce(mockTsvContent) - const result = await loadDataFile('values.tsv') + const result = await loadDataFiles(['values.tsv']) expect(result).toStrictEqual([ - { acc: 0.69, epoch: 10 }, - { acc: 0.345, epoch: 11 } + { + data: [ + { acc: 0.69, epoch: 10 }, + { acc: 0.345, epoch: 11 } + ], + file: 'values.tsv' + } ]) }) @@ -115,15 +130,20 @@ describe('loadDataFile', () => { mockedReadFileSync.mockReturnValueOnce(mockYamlContent) - const result = await loadDataFile('dvc.yaml') + const result = await loadDataFiles(['dvc.yaml']) - expect(result).toStrictEqual({ - stages: { - train: { - cmd: 'python train.py' - } + expect(result).toStrictEqual([ + { + data: { + stages: { + train: { + cmd: 'python train.py' + } + } + }, + file: 'dvc.yaml' } - }) + ]) }) it('should catch any errors thrown during file parsing', async () => { @@ -133,7 +153,7 @@ describe('loadDataFile', () => { }) for (const file of dataFiles) { - const resultWithErr = await loadDataFile(file) + const resultWithErr = await loadDataFiles([file]) expect(resultWithErr).toStrictEqual(undefined) } @@ -541,10 +561,9 @@ describe('addPlotToDvcYamlFile', () => { mockedReadFileSync.mockReturnValueOnce(mockDvcYamlContent) addPlotToDvcYamlFile('/', { - dataFile: '/data.json', template: 'simple', - x: 'epochs', - y: 'accuracy' + x: { file: '/data.json', key: 'epochs' }, + y: { file: '/data.json', key: 'accuracy' } }) expect(mockedWriteFileSync).toHaveBeenCalledWith( @@ -560,10 +579,9 @@ describe('addPlotToDvcYamlFile', () => { mockedReadFileSync.mockReturnValueOnce(mockDvcYamlContent.join('\n')) addPlotToDvcYamlFile('/', { - dataFile: '/data.json', template: 'simple', - x: 'epochs', - y: 'accuracy' + x: { file: '/data.json', key: 'epochs' }, + y: { file: '/data.json', key: 'accuracy' } }) mockDvcYamlContent.splice(7, 0, ...mockPlotYamlContent) @@ -583,10 +601,9 @@ describe('addPlotToDvcYamlFile', () => { mockedReadFileSync.mockReturnValueOnce(mockDvcYamlContent) addPlotToDvcYamlFile('/', { - dataFile: '/data.json', template: 'simple', - x: 'epochs', - y: 'accuracy' + x: { file: '/data.json', key: 'epochs' }, + y: { file: '/data.json', key: 'accuracy' } }) expect(mockedWriteFileSync).toHaveBeenCalledWith( @@ -620,10 +637,9 @@ describe('addPlotToDvcYamlFile', () => { mockedReadFileSync.mockReturnValueOnce(mockDvcYamlContent) addPlotToDvcYamlFile('/', { - dataFile: '/data.json', template: 'simple', - x: 'epochs', - y: 'accuracy' + x: { file: '/data.json', key: 'epochs' }, + y: { file: '/data.json', key: 'accuracy' } }) expect(mockedWriteFileSync).toHaveBeenCalledWith( diff --git a/extension/src/fileSystem/index.ts b/extension/src/fileSystem/index.ts index 2ddfee9289..4e32be1f95 100644 --- a/extension/src/fileSystem/index.ts +++ b/extension/src/fileSystem/index.ts @@ -214,13 +214,30 @@ const loadYamlAsDoc = ( } } +const getPlotYamlObj = (cwd: string, plot: PlotConfigData) => { + const { x, y, template } = plot + const usesSingleFile = x.file === y.file + if (usesSingleFile) { + const dataFile = x.file + const plotName = relative(cwd, dataFile) + return { [plotName]: { template, x: x.key, y: y.key } } + } + + const plotName = `${template}_plot` + return { + [plotName]: { + template, + x: { [relative(cwd, x.file)]: x.key }, + y: { [relative(cwd, y.file)]: y.key } + } + } +} + const getPlotsYaml = ( cwd: string, plotObj: PlotConfigData, indentSearchLines: string[] ) => { - const { dataFile, ...plot } = plotObj - const plotName = relative(cwd, dataFile) const indentReg = /^( +)[^ ]/ const indentLine = indentSearchLines.find(line => indentReg.test(line)) || '' const spacesMatches = indentLine.match(indentReg) @@ -228,7 +245,7 @@ const getPlotsYaml = ( return yaml .stringify( - { plots: [{ [plotName]: plot }] }, + { plots: [getPlotYamlObj(cwd, plotObj)] }, { indent: spaces ? spaces.length : 2 } ) .split('\n') @@ -315,7 +332,7 @@ const loadTsv = (path: string) => { } } -export const loadDataFile = (file: string): unknown => { +const loadDataFile = (file: string): unknown => { const ext = getFileExtension(file) switch (ext) { @@ -330,6 +347,22 @@ export const loadDataFile = (file: string): unknown => { } } +export const loadDataFiles = async ( + files: string[] +): Promise<{ file: string; data: unknown }[] | undefined> => { + const filesData: { file: string; data: unknown }[] = [] + for (const file of files) { + const data = await loadDataFile(file) + + if (!data) { + return undefined + } + + filesData.push({ data, file }) + } + return filesData +} + export const writeJson = < T extends Record | Array> >( diff --git a/extension/src/fileSystem/util.ts b/extension/src/fileSystem/util.ts index 36ab159925..53effd7ac0 100644 --- a/extension/src/fileSystem/util.ts +++ b/extension/src/fileSystem/util.ts @@ -22,4 +22,6 @@ export const getParent = (pathArray: string[], idx: number) => { export const removeTrailingSlash = (path: string): string => path.endsWith(sep) ? path.slice(0, -1) : path +export const getFileName = (path: string) => parse(path).base + export const getFileNameWithoutExt = (path: string) => parse(path).name diff --git a/extension/src/pipeline/quickPick.test.ts b/extension/src/pipeline/quickPick.test.ts index e43baab160..51d9864113 100644 --- a/extension/src/pipeline/quickPick.test.ts +++ b/extension/src/pipeline/quickPick.test.ts @@ -1,14 +1,16 @@ +import { QuickPickItemKind } from 'vscode' import { pickPlotConfiguration } from './quickPick' -import { pickFile } from '../vscode/resourcePicker' -import { quickPickOne } from '../vscode/quickPick' -import { getFileExtension, loadDataFile } from '../fileSystem' +import { pickFiles } from '../vscode/resourcePicker' +import { quickPickOne, quickPickValue } from '../vscode/quickPick' +import { getFileExtension, loadDataFiles } from '../fileSystem' import { Title } from '../vscode/title' import { Toast } from '../vscode/toast' -const mockedPickFile = jest.mocked(pickFile) -const mockedLoadDataFile = jest.mocked(loadDataFile) +const mockedPickFiles = jest.mocked(pickFiles) +const mockedLoadDataFiles = jest.mocked(loadDataFiles) const mockedGetFileExt = jest.mocked(getFileExtension) const mockedQuickPickOne = jest.mocked(quickPickOne) +const mockedQuickPickValue = jest.mocked(quickPickValue) const mockedToast = jest.mocked(Toast) const mockedShowError = jest.fn() mockedToast.showError = mockedShowError @@ -47,18 +49,20 @@ const mockValidData = [ describe('pickPlotConfiguration', () => { it('should let the user pick from files with accepted data types', async () => { - mockedPickFile.mockResolvedValueOnce('file.json') - mockedLoadDataFile.mockReturnValueOnce(mockValidData) + mockedPickFiles.mockResolvedValueOnce(['file.json']) + mockedLoadDataFiles.mockResolvedValueOnce([ + { data: mockValidData, file: 'file.json' } + ]) await pickPlotConfiguration() - expect(mockedPickFile).toHaveBeenCalledWith(Title.SELECT_PLOT_DATA, { + expect(mockedPickFiles).toHaveBeenCalledWith(Title.SELECT_PLOT_DATA, { 'Data Formats': ['json', 'csv', 'tsv', 'yaml'] }) }) it('should return early if user does not select a file', async () => { - mockedPickFile.mockResolvedValueOnce(undefined) + mockedPickFiles.mockResolvedValueOnce(undefined) const result = await pickPlotConfiguration() @@ -66,20 +70,20 @@ describe('pickPlotConfiguration', () => { }) it('should show a toast message if the file fails to parse', async () => { - mockedPickFile.mockResolvedValueOnce('file.csv') - mockedLoadDataFile.mockReturnValueOnce(undefined) + mockedPickFiles.mockResolvedValueOnce(['data.csv']) + mockedLoadDataFiles.mockResolvedValueOnce(undefined) const result = await pickPlotConfiguration() expect(result).toStrictEqual(undefined) expect(mockedShowError).toHaveBeenCalledTimes(1) expect(mockedShowError).toHaveBeenCalledWith( - 'Failed to parse the requested file. Does the file contain data and follow the DVC plot guidelines for [JSON/YAML](https://dvc.org/doc/command-reference/plots/show#example-hierarchical-data) or [CSV/TSV](https://dvc.org/doc/command-reference/plots/show#example-tabular-data) files?' + 'Failed to parse the requested file(s). Does the file or files contain data and follow the DVC plot guidelines for [JSON/YAML](https://dvc.org/doc/command-reference/plots/show#example-hierarchical-data) or [CSV/TSV](https://dvc.org/doc/command-reference/plots/show#example-tabular-data) files?' ) }) it('should show a toast message if an array of objects (with atleast two keys) are not found within a file', async () => { - mockedPickFile.mockResolvedValue('file.yaml') + mockedPickFiles.mockResolvedValue(['file.yaml']) const invalidValues: unknown[] = [ 'string', 13, @@ -115,20 +119,22 @@ describe('pickPlotConfiguration', () => { let result for (const [ind, invalidVal] of invalidValues.entries()) { - mockedLoadDataFile.mockReturnValueOnce(invalidVal) + mockedLoadDataFiles.mockResolvedValueOnce([ + { data: invalidVal, file: 'file.yaml' } + ]) result = await pickPlotConfiguration() expect(result).toStrictEqual(undefined) expect(mockedShowError).toHaveBeenCalledTimes(1 + ind) expect(mockedShowError).toHaveBeenCalledWith( - 'The requested file does not contain enough keys (columns) to generate a plot. Does the file follow the DVC plot guidelines for [JSON/YAML](https://dvc.org/doc/command-reference/plots/show#example-hierarchical-data) or [CSV/TSV](https://dvc.org/doc/command-reference/plots/show#example-tabular-data) files?' + 'The requested file(s) does not contain enough keys (columns) to generate a plot. Does the file or files follow the DVC plot guidelines for [JSON/YAML](https://dvc.org/doc/command-reference/plots/show#example-hierarchical-data) or [CSV/TSV](https://dvc.org/doc/command-reference/plots/show#example-tabular-data) files?' ) } }) it('should parse fields from valid data files', async () => { - mockedPickFile.mockResolvedValue('file.yaml') + mockedPickFiles.mockResolvedValue(['file.yaml']) const validValues: unknown[] = [ [{ field1: 1, field2: 2 }], [ @@ -145,7 +151,9 @@ describe('pickPlotConfiguration', () => { ] for (const [ind, val] of validValues.entries()) { - mockedLoadDataFile.mockReturnValueOnce(val) + mockedLoadDataFiles.mockResolvedValueOnce([ + { data: val, file: 'file.yaml' } + ]) await pickPlotConfiguration() @@ -155,12 +163,14 @@ describe('pickPlotConfiguration', () => { }) it('should let the user pick a template, x field, and y field', async () => { - mockedPickFile.mockResolvedValueOnce('file.json') - mockedLoadDataFile.mockReturnValueOnce(mockValidData) - mockedQuickPickOne - .mockResolvedValueOnce('simple') - .mockResolvedValueOnce('actual') - .mockResolvedValueOnce('prob') + mockedPickFiles.mockResolvedValueOnce(['file.json']) + mockedLoadDataFiles.mockResolvedValueOnce([ + { data: mockValidData, file: 'file.json' } + ]) + mockedQuickPickOne.mockResolvedValueOnce('simple') + mockedQuickPickValue + .mockResolvedValueOnce({ file: 'file.json', key: 'actual' }) + .mockResolvedValueOnce({ file: 'file.json', key: 'prob' }) const result = await pickPlotConfiguration() @@ -179,27 +189,45 @@ describe('pickPlotConfiguration', () => { ], 'Pick a Plot Template' ) - expect(mockedQuickPickOne).toHaveBeenNthCalledWith( - 2, - ['actual', 'prob'], - 'Pick a Metric for X' + expect(mockedQuickPickValue).toHaveBeenNthCalledWith( + 1, + [ + { + kind: QuickPickItemKind.Separator, + label: 'file.json', + value: undefined + }, + { label: 'actual', value: { file: 'file.json', key: 'actual' } }, + { label: 'prob', value: { file: 'file.json', key: 'prob' } } + ], + { title: Title.SELECT_PLOT_X_METRIC } ) - expect(mockedQuickPickOne).toHaveBeenNthCalledWith( - 3, - ['prob'], - 'Pick a Metric for Y' + expect(mockedQuickPickValue).toHaveBeenNthCalledWith( + 2, + [ + { + kind: QuickPickItemKind.Separator, + label: 'file.json', + value: undefined + }, + { label: 'prob', value: { file: 'file.json', key: 'prob' } } + ], + { + title: Title.SELECT_PLOT_Y_METRIC + } ) expect(result).toStrictEqual({ - dataFile: 'file.json', template: 'simple', - x: 'actual', - y: 'prob' + x: { file: 'file.json', key: 'actual' }, + y: { file: 'file.json', key: 'prob' } }) }) it('should return early if the user does not pick a template', async () => { - mockedPickFile.mockResolvedValueOnce('file.json') - mockedLoadDataFile.mockReturnValueOnce(mockValidData) + mockedPickFiles.mockResolvedValueOnce(['file.json']) + mockedLoadDataFiles.mockResolvedValueOnce([ + { data: mockValidData, file: 'file.json' } + ]) mockedQuickPickOne.mockResolvedValueOnce(undefined) const result = await pickPlotConfiguration() @@ -210,29 +238,32 @@ describe('pickPlotConfiguration', () => { }) it('should return early if the user does not pick a x field', async () => { - mockedPickFile.mockResolvedValueOnce('file.json') - mockedLoadDataFile.mockReturnValueOnce(mockValidData) - mockedQuickPickOne - .mockResolvedValueOnce('simple') - .mockResolvedValueOnce(undefined) + mockedPickFiles.mockResolvedValueOnce(['file.json']) + mockedLoadDataFiles.mockResolvedValueOnce([ + { data: mockValidData, file: 'file.json' } + ]) + mockedQuickPickOne.mockResolvedValueOnce('simple') + mockedQuickPickValue.mockResolvedValueOnce(undefined) const result = await pickPlotConfiguration() - expect(mockedQuickPickOne).toHaveBeenCalledTimes(2) + expect(mockedQuickPickValue).toHaveBeenCalledTimes(1) expect(result).toStrictEqual(undefined) }) it('should return early if the user does not pick a y field', async () => { - mockedPickFile.mockResolvedValueOnce('file.json') - mockedLoadDataFile.mockReturnValueOnce(mockValidData) - mockedQuickPickOne - .mockResolvedValueOnce('simple') + mockedPickFiles.mockResolvedValueOnce(['file.json']) + mockedLoadDataFiles.mockResolvedValueOnce([ + { data: mockValidData, file: 'file.json' } + ]) + mockedQuickPickOne.mockResolvedValueOnce('simple') + mockedQuickPickValue .mockResolvedValueOnce('actual') .mockResolvedValueOnce(undefined) const result = await pickPlotConfiguration() - expect(mockedQuickPickOne).toHaveBeenCalledTimes(3) + expect(mockedQuickPickValue).toHaveBeenCalledTimes(2) expect(result).toStrictEqual(undefined) }) }) diff --git a/extension/src/pipeline/quickPick.ts b/extension/src/pipeline/quickPick.ts index fd03adf667..5fda8e3b9e 100644 --- a/extension/src/pipeline/quickPick.ts +++ b/extension/src/pipeline/quickPick.ts @@ -1,40 +1,54 @@ import isEqual from 'lodash.isequal' +import { QuickPickItemKind } from 'vscode' import { PLOT_TEMPLATES, Value, ValueTree, isValueTree } from '../cli/dvc/contract' -import { loadDataFile } from '../fileSystem' -import { quickPickOne } from '../vscode/quickPick' -import { pickFile } from '../vscode/resourcePicker' +import { loadDataFiles } from '../fileSystem' +import { quickPickOne, quickPickValue } from '../vscode/quickPick' +import { pickFiles } from '../vscode/resourcePicker' import { Title } from '../vscode/title' import { Toast } from '../vscode/toast' +import { getFileName } from '../fileSystem/util' -const pickDataFile = () => { - return pickFile(Title.SELECT_PLOT_DATA, { +const pickDataFiles = (): Thenable => + pickFiles(Title.SELECT_PLOT_DATA, { 'Data Formats': ['json', 'csv', 'tsv', 'yaml'] }) -} -const pickTemplateAndFields = async ( - fields: string[] -): Promise<{ x: string; y: string; template: string } | undefined> => { +const pickTemplateAndFields = async (fields: { + [file: string]: string[] +}): Promise => { const template = await quickPickOne(PLOT_TEMPLATES, 'Pick a Plot Template') if (!template) { return } - const x = await quickPickOne(fields, 'Pick a Metric for X') + const items = [] + + for (const [file, keys] of Object.entries(fields)) { + items.push( + { + kind: QuickPickItemKind.Separator, + label: getFileName(file), + value: undefined + }, + ...keys.map(key => ({ label: key, value: { file, key } })) + ) + } + + const x = await quickPickValue(items, { title: Title.SELECT_PLOT_X_METRIC }) if (!x) { return } - const y = await quickPickOne( - fields.filter(field => x !== field), - 'Pick a Metric for Y' + const y = await quickPickValue( + items.filter(item => item.value?.key !== x.key), + { title: Title.SELECT_PLOT_Y_METRIC } ) if (!y) { @@ -45,10 +59,9 @@ const pickTemplateAndFields = async ( } export type PlotConfigData = { - dataFile: string + x: { file: string; key: string } template: string - x: string - y: string + y: { file: string; key: string } } type UnknownValue = Value | ValueTree @@ -79,28 +92,56 @@ const getFieldOptions = (data: UnknownValue): string[] => { : [] } +const getFieldOptionsFromArr = ( + dataArr: { data: UnknownValue; file: string }[] +) => { + const keys: { + [file: string]: string[] + } = {} + let keysAmount = 0 + + for (const { file, data } of dataArr) { + const fields = getFieldOptions(data) + + if (fields.length === 0) { + continue + } + + keysAmount += fields.length + + if (!keys[file]) { + keys[file] = [] + } + keys[file].push(...fields) + } + + return { keys, keysAmount } +} + export const pickPlotConfiguration = async (): Promise< PlotConfigData | undefined > => { - const file = await pickDataFile() + const files = await pickDataFiles() - if (!file) { + if (!files) { return } - const data = await loadDataFile(file) + const filesData = await loadDataFiles(files) - if (!data) { + if (!filesData) { return Toast.showError( - 'Failed to parse the requested file. Does the file contain data and follow the DVC plot guidelines for [JSON/YAML](https://dvc.org/doc/command-reference/plots/show#example-hierarchical-data) or [CSV/TSV](https://dvc.org/doc/command-reference/plots/show#example-tabular-data) files?' + 'Failed to parse the requested file(s). Does the file or files contain data and follow the DVC plot guidelines for [JSON/YAML](https://dvc.org/doc/command-reference/plots/show#example-hierarchical-data) or [CSV/TSV](https://dvc.org/doc/command-reference/plots/show#example-tabular-data) files?' ) } - const keys = getFieldOptions(data as UnknownValue) + const { keys, keysAmount } = getFieldOptionsFromArr( + filesData as { data: UnknownValue; file: string }[] + ) - if (keys.length < 2) { + if (keysAmount < 2) { return Toast.showError( - 'The requested file does not contain enough keys (columns) to generate a plot. Does the file follow the DVC plot guidelines for [JSON/YAML](https://dvc.org/doc/command-reference/plots/show#example-hierarchical-data) or [CSV/TSV](https://dvc.org/doc/command-reference/plots/show#example-tabular-data) files?' + 'The requested file(s) does not contain enough keys (columns) to generate a plot. Does the file or files follow the DVC plot guidelines for [JSON/YAML](https://dvc.org/doc/command-reference/plots/show#example-hierarchical-data) or [CSV/TSV](https://dvc.org/doc/command-reference/plots/show#example-tabular-data) files?' ) } @@ -110,5 +151,5 @@ export const pickPlotConfiguration = async (): Promise< return } - return { ...templateAndFields, dataFile: file } + return { ...templateAndFields } } diff --git a/extension/src/test/suite/pipeline/index.test.ts b/extension/src/test/suite/pipeline/index.test.ts index bde79ed3ff..9ba5a92915 100644 --- a/extension/src/test/suite/pipeline/index.test.ts +++ b/extension/src/test/suite/pipeline/index.test.ts @@ -310,10 +310,9 @@ suite('Pipeline Test Suite', () => { expect(mockAddPlotToDvcFile).not.to.be.called mockPickPlotConfiguration.onSecondCall().resolves({ - dataFile: 'results.json', template: 'simple', - x: 'step', - y: 'acc' + x: { file: 'results.json', key: 'step' }, + y: { file: 'results.json', key: 'acc' } }) await pipeline.addDataSeriesPlot() diff --git a/extension/src/vscode/resourcePicker.ts b/extension/src/vscode/resourcePicker.ts index 43b6b5aa15..7d47efb25f 100644 --- a/extension/src/vscode/resourcePicker.ts +++ b/extension/src/vscode/resourcePicker.ts @@ -25,12 +25,30 @@ export const pickFile = async ( } } -export const pickResources = (title: Title): Thenable => { - return window.showOpenDialog({ +export const pickResources = ( + title: Title, + filters?: OpenDialogOptions['filters'] +): Thenable => { + const opts: OpenDialogOptions = { canSelectFiles: true, canSelectFolders: true, canSelectMany: true, openLabel: 'Select', title - }) + } + + if (!filters) { + opts.filters = filters + } + + return window.showOpenDialog(opts) +} + +export const pickFiles = async ( + title: Title, + filters?: OpenDialogOptions['filters'] +): Promise => { + const files = await pickResources(title, filters) + + return files?.map(({ fsPath }) => fsPath) } diff --git a/extension/src/vscode/title.ts b/extension/src/vscode/title.ts index d9fbbf486b..1545c25e83 100644 --- a/extension/src/vscode/title.ts +++ b/extension/src/vscode/title.ts @@ -40,6 +40,8 @@ export enum Title { SELECT_SORT_DIRECTION = 'Select Sort Direction', SELECT_SORTS_TO_REMOVE = 'Select Sort(s) to Remove', SELECT_TRAINING_SCRIPT = 'Select your Training Script', + SELECT_PLOT_X_METRIC = 'Select a Metric for X', + SELECT_PLOT_Y_METRIC = 'Select a Metric for Y', SELECT_PLOT_DATA = 'Select Data to Plot', SELECT_PLOT_TYPE = 'Select Plot Type', SETUP_WORKSPACE = 'Setup the Workspace', From 68953fc6b4654094caf6f2c2be2505da5a98e6e4 Mon Sep 17 00:00:00 2001 From: julieg18 Date: Mon, 2 Oct 2023 11:39:33 -0500 Subject: [PATCH 02/12] adjust filter --- extension/src/pipeline/quickPick.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/extension/src/pipeline/quickPick.ts b/extension/src/pipeline/quickPick.ts index 5fda8e3b9e..7b5eb2f814 100644 --- a/extension/src/pipeline/quickPick.ts +++ b/extension/src/pipeline/quickPick.ts @@ -47,7 +47,9 @@ const pickTemplateAndFields = async (fields: { } const y = await quickPickValue( - items.filter(item => item.value?.key !== x.key), + items.filter( + item => item.value?.key !== x.key && item.value?.file !== x.file + ), { title: Title.SELECT_PLOT_Y_METRIC } ) From e410b537fb500e336e6a749c58c434a46e0a23e0 Mon Sep 17 00:00:00 2001 From: julieg18 Date: Mon, 2 Oct 2023 18:17:52 -0500 Subject: [PATCH 03/12] Add new tests --- extension/src/fileSystem/index.test.ts | 60 ++++++++++++ extension/src/fileSystem/index.ts | 4 +- extension/src/pipeline/quickPick.test.ts | 120 ++++++++++++++++++++++- extension/src/pipeline/quickPick.ts | 4 +- 4 files changed, 180 insertions(+), 8 deletions(-) diff --git a/extension/src/fileSystem/index.test.ts b/extension/src/fileSystem/index.test.ts index 0c39f28a9c..ac1ebf0663 100644 --- a/extension/src/fileSystem/index.test.ts +++ b/extension/src/fileSystem/index.test.ts @@ -146,6 +146,33 @@ describe('loadDataFiles', () => { ]) }) + it('should load in the contents of multiple files', async () => { + const mockTsvContent = ['epoch\tacc', '10\t0.69', '11\t0.345'].join('\n') + const mockCsvContent = ['epoch2,acc2', '10,0.679', '11,0.3'].join('\n') + + mockedReadFileSync.mockReturnValueOnce(mockTsvContent) + mockedReadFileSync.mockReturnValueOnce(mockCsvContent) + + const result = await loadDataFiles(['values.tsv', 'values2.csv']) + + expect(result).toStrictEqual([ + { + data: [ + { acc: 0.69, epoch: 10 }, + { acc: 0.345, epoch: 11 } + ], + file: 'values.tsv' + }, + { + data: [ + { acc2: 0.679, epoch2: 10 }, + { acc2: 0.3, epoch2: 11 } + ], + file: 'values2.csv' + } + ]) + }) + it('should catch any errors thrown during file parsing', async () => { const dataFiles = ['values.csv', 'file.json', 'file.tsv', 'dvc.yaml'] mockedReadFileSync.mockImplementation(() => { @@ -158,6 +185,39 @@ describe('loadDataFiles', () => { expect(resultWithErr).toStrictEqual(undefined) } }) + + it('should catch any errors thrown during the parsing of multiple files', async () => { + const dataFiles = ['values.csv', 'file.tsv', 'file.json'] + const mockCsvContent = ['epoch,acc', '10,0.69', '11,0.345'].join('\n') + const mockJsonContent = JSON.stringify([ + { acc: 0.69, epoch: 10 }, + { acc: 0.345, epoch: 11 } + ]) + mockedReadFileSync + .mockReturnValueOnce(mockCsvContent) + .mockImplementationOnce(() => { + throw new Error('fake error') + }) + .mockReturnValueOnce(mockJsonContent) + + const resultWithErr = await loadDataFiles(dataFiles) + expect(resultWithErr).toStrictEqual([ + { + data: [ + { acc: 0.69, epoch: 10 }, + { acc: 0.345, epoch: 11 } + ], + file: 'values.csv' + }, + { + data: [ + { acc: 0.69, epoch: 10 }, + { acc: 0.345, epoch: 11 } + ], + file: 'file.json' + } + ]) + }) }) describe('writeJson', () => { diff --git a/extension/src/fileSystem/index.ts b/extension/src/fileSystem/index.ts index 4e32be1f95..f6bf8e50c3 100644 --- a/extension/src/fileSystem/index.ts +++ b/extension/src/fileSystem/index.ts @@ -355,12 +355,12 @@ export const loadDataFiles = async ( const data = await loadDataFile(file) if (!data) { - return undefined + continue } filesData.push({ data, file }) } - return filesData + return filesData.length === 0 ? undefined : filesData } export const writeJson = < diff --git a/extension/src/pipeline/quickPick.test.ts b/extension/src/pipeline/quickPick.test.ts index 51d9864113..900a4c9cc7 100644 --- a/extension/src/pipeline/quickPick.test.ts +++ b/extension/src/pipeline/quickPick.test.ts @@ -69,7 +69,7 @@ describe('pickPlotConfiguration', () => { expect(result).toStrictEqual(undefined) }) - it('should show a toast message if the file fails to parse', async () => { + it('should show a toast message if files(s) fail to parse', async () => { mockedPickFiles.mockResolvedValueOnce(['data.csv']) mockedLoadDataFiles.mockResolvedValueOnce(undefined) @@ -82,7 +82,7 @@ describe('pickPlotConfiguration', () => { ) }) - it('should show a toast message if an array of objects (with atleast two keys) are not found within a file', async () => { + it('should show a toast message if an array of objects (with atleast two keys total) are not found within a file', async () => { mockedPickFiles.mockResolvedValue(['file.yaml']) const invalidValues: unknown[] = [ 'string', @@ -133,7 +133,28 @@ describe('pickPlotConfiguration', () => { } }) - it('should parse fields from valid data files', async () => { + it('should show a toast message if atleast two fields are not found with multiple files', async () => { + mockedPickFiles.mockResolvedValueOnce([ + 'file.yaml', + 'file2.yaml', + 'file3.yaml' + ]) + mockedLoadDataFiles.mockResolvedValueOnce([ + { data: '', file: 'file.yaml' }, + { data: { val: [{ field1: 'only one field' }] }, file: 'file2.yaml' }, + { data: [], file: 'file3.yaml' } + ]) + + const result = await pickPlotConfiguration() + + expect(result).toStrictEqual(undefined) + expect(mockedShowError).toHaveBeenCalledTimes(1) + expect(mockedShowError).toHaveBeenCalledWith( + 'The requested file(s) does not contain enough keys (columns) to generate a plot. Does the file or files follow the DVC plot guidelines for [JSON/YAML](https://dvc.org/doc/command-reference/plots/show#example-hierarchical-data) or [CSV/TSV](https://dvc.org/doc/command-reference/plots/show#example-tabular-data) files?' + ) + }) + + it('should parse fields from a valid data file', async () => { mockedPickFiles.mockResolvedValue(['file.yaml']) const validValues: unknown[] = [ [{ field1: 1, field2: 2 }], @@ -162,6 +183,38 @@ describe('pickPlotConfiguration', () => { } }) + it('should parse fields from a mixture of valid and invalid files if atleast 2 fields are found in total', async () => { + mockedPickFiles.mockResolvedValueOnce([ + 'file.yaml', + 'file2.yaml', + 'file3.yaml' + ]) + mockedLoadDataFiles.mockResolvedValueOnce([ + { + data: [ + { field1: 1, field2: 2, field3: 1, field4: 2 }, + { field1: 1, field2: 2, field3: 1, field4: 2 } + ], + file: 'file.yaml' + }, + { + data: { + field1: [{ field1: 1 }, { field1: 3 }] + }, + file: 'file3.yaml' + }, + { + data: undefined, + file: 'file3.yaml' + } + ]) + + await pickPlotConfiguration() + + expect(mockedShowError).not.toHaveBeenCalled() + expect(mockedQuickPickOne).toHaveBeenCalledTimes(1) + }) + it('should let the user pick a template, x field, and y field', async () => { mockedPickFiles.mockResolvedValueOnce(['file.json']) mockedLoadDataFiles.mockResolvedValueOnce([ @@ -223,6 +276,67 @@ describe('pickPlotConfiguration', () => { }) }) + it('should let the user pick a x field and y field from multiple files', async () => { + mockedPickFiles.mockResolvedValueOnce(['file.json', 'file2.json']) + mockedLoadDataFiles.mockResolvedValueOnce([ + { data: mockValidData, file: 'file.json' }, + { data: mockValidData, file: 'file2.json' } + ]) + mockedQuickPickOne.mockResolvedValueOnce('simple') + mockedQuickPickValue + .mockResolvedValueOnce({ file: 'file.json', key: 'actual' }) + .mockResolvedValueOnce({ file: 'file2.json', key: 'prob' }) + + const result = await pickPlotConfiguration() + + expect(mockedQuickPickValue).toHaveBeenNthCalledWith( + 1, + [ + { + kind: QuickPickItemKind.Separator, + label: 'file.json', + value: undefined + }, + { label: 'actual', value: { file: 'file.json', key: 'actual' } }, + { label: 'prob', value: { file: 'file.json', key: 'prob' } }, + { + kind: QuickPickItemKind.Separator, + label: 'file2.json', + value: undefined + }, + { label: 'actual', value: { file: 'file2.json', key: 'actual' } }, + { label: 'prob', value: { file: 'file2.json', key: 'prob' } } + ], + { title: Title.SELECT_PLOT_X_METRIC } + ) + expect(mockedQuickPickValue).toHaveBeenNthCalledWith( + 2, + [ + { + kind: QuickPickItemKind.Separator, + label: 'file.json', + value: undefined + }, + { label: 'prob', value: { file: 'file.json', key: 'prob' } }, + { + kind: QuickPickItemKind.Separator, + label: 'file2.json', + value: undefined + }, + { label: 'actual', value: { file: 'file2.json', key: 'actual' } }, + { label: 'prob', value: { file: 'file2.json', key: 'prob' } } + ], + { + title: Title.SELECT_PLOT_Y_METRIC + } + ) + expect(result).toStrictEqual({ + template: 'simple', + x: { file: 'file.json', key: 'actual' }, + y: { file: 'file2.json', key: 'prob' } + }) + }) + it('should return early if the user does not pick a template', async () => { mockedPickFiles.mockResolvedValueOnce(['file.json']) mockedLoadDataFiles.mockResolvedValueOnce([ diff --git a/extension/src/pipeline/quickPick.ts b/extension/src/pipeline/quickPick.ts index 7b5eb2f814..c131003853 100644 --- a/extension/src/pipeline/quickPick.ts +++ b/extension/src/pipeline/quickPick.ts @@ -47,9 +47,7 @@ const pickTemplateAndFields = async (fields: { } const y = await quickPickValue( - items.filter( - item => item.value?.key !== x.key && item.value?.file !== x.file - ), + items.filter(item => !isEqual(item.value, x)), { title: Title.SELECT_PLOT_Y_METRIC } ) From 1a86d95786852ac445c31a80fc98ad2c949aa8fc Mon Sep 17 00:00:00 2001 From: julieg18 Date: Mon, 2 Oct 2023 18:23:13 -0500 Subject: [PATCH 04/12] Add missing test for dvc.yaml --- extension/src/fileSystem/index.test.ts | 28 ++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/extension/src/fileSystem/index.test.ts b/extension/src/fileSystem/index.test.ts index ac1ebf0663..5e4bc8c2d5 100644 --- a/extension/src/fileSystem/index.test.ts +++ b/extension/src/fileSystem/index.test.ts @@ -632,6 +632,34 @@ describe('addPlotToDvcYamlFile', () => { ) }) + it('should add the new plot with fields coming from different fiels', () => { + const mockDvcYamlContent = mockStagesLines.join('\n') + const mockPlotYamlContent = [ + '', + 'plots:', + ' - simple_plot:', + ' template: simple', + ' x:', + ' data.json: epochs', + ' y:', + ' acc.json: accuracy', + '' + ].join('\n') + mockedReadFileSync.mockReturnValueOnce(mockDvcYamlContent) + mockedReadFileSync.mockReturnValueOnce(mockDvcYamlContent) + + addPlotToDvcYamlFile('/', { + template: 'simple', + x: { file: '/data.json', key: 'epochs' }, + y: { file: '/acc.json', key: 'accuracy' } + }) + + expect(mockedWriteFileSync).toHaveBeenCalledWith( + '//dvc.yaml', + mockDvcYamlContent + mockPlotYamlContent + ) + }) + it('should add the new plot if the dvc.yaml file already has plots', () => { const mockDvcYamlContent = [...mockPlotsListLines, ...mockStagesLines] const mockPlotYamlContent = [...mockNewPlotLines, ''] From f81ef2f109c40527359a338adef1fcfe630f1d18 Mon Sep 17 00:00:00 2001 From: julieg18 Date: Mon, 2 Oct 2023 18:33:53 -0500 Subject: [PATCH 05/12] resourcePicker adjustments --- extension/src/vscode/resourcePicker.test.ts | 15 ++++++++++++++- extension/src/vscode/resourcePicker.ts | 10 ++-------- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/extension/src/vscode/resourcePicker.test.ts b/extension/src/vscode/resourcePicker.test.ts index 23256ffc5f..38ae071e40 100644 --- a/extension/src/vscode/resourcePicker.test.ts +++ b/extension/src/vscode/resourcePicker.test.ts @@ -1,6 +1,6 @@ import { resolve } from 'path' import { Uri, window } from 'vscode' -import { pickFile, pickResources } from './resourcePicker' +import { pickFile, pickResources, pickFiles } from './resourcePicker' import { Title } from './title' jest.mock('vscode') @@ -67,3 +67,16 @@ describe('pickResources', () => { expect(pickedResources).toStrictEqual([mockedUri]) }) }) + +describe('pickFiles', () => { + it('should return an array of selected file paths', async () => { + const files = [resolve('mock', 'file'), resolve('mock', 'file2')] + const mockedPickedUris = files.map(file => Uri.file(file)) + const mockedTitle = 'insert great title here' as Title + mockedShowOpenDialog.mockResolvedValueOnce(mockedPickedUris) + + const pickedResources = await pickFiles(mockedTitle) + + expect(pickedResources).toStrictEqual(files) + }) +}) diff --git a/extension/src/vscode/resourcePicker.ts b/extension/src/vscode/resourcePicker.ts index 7d47efb25f..651a6b98f7 100644 --- a/extension/src/vscode/resourcePicker.ts +++ b/extension/src/vscode/resourcePicker.ts @@ -5,19 +5,13 @@ export const pickFile = async ( title: Title, filters?: OpenDialogOptions['filters'] ): Promise => { - const opts: OpenDialogOptions = { + const uris = await window.showOpenDialog({ canSelectFolders: false, canSelectMany: false, filters, openLabel: 'Select', title - } - - if (filters) { - opts.filters = filters - } - - const uris = await window.showOpenDialog(opts) + }) if (uris) { const [{ fsPath }] = uris From a6b4a91b9ff252a3e3ecfc9b7d9c8074184cdce0 Mon Sep 17 00:00:00 2001 From: julieg18 Date: Tue, 3 Oct 2023 08:25:36 -0500 Subject: [PATCH 06/12] Adjust how we handle failed files --- extension/src/fileSystem/index.test.ts | 19 ++--------- extension/src/fileSystem/index.ts | 4 +-- extension/src/pipeline/quickPick.test.ts | 37 ++++++--------------- extension/src/pipeline/quickPick.ts | 2 +- extension/src/vscode/resourcePicker.test.ts | 10 +++--- 5 files changed, 22 insertions(+), 50 deletions(-) diff --git a/extension/src/fileSystem/index.test.ts b/extension/src/fileSystem/index.test.ts index 5e4bc8c2d5..90fa5be5ee 100644 --- a/extension/src/fileSystem/index.test.ts +++ b/extension/src/fileSystem/index.test.ts @@ -201,22 +201,7 @@ describe('loadDataFiles', () => { .mockReturnValueOnce(mockJsonContent) const resultWithErr = await loadDataFiles(dataFiles) - expect(resultWithErr).toStrictEqual([ - { - data: [ - { acc: 0.69, epoch: 10 }, - { acc: 0.345, epoch: 11 } - ], - file: 'values.csv' - }, - { - data: [ - { acc: 0.69, epoch: 10 }, - { acc: 0.345, epoch: 11 } - ], - file: 'file.json' - } - ]) + expect(resultWithErr).toStrictEqual(undefined) }) }) @@ -632,7 +617,7 @@ describe('addPlotToDvcYamlFile', () => { ) }) - it('should add the new plot with fields coming from different fiels', () => { + it('should add the new plot with fields coming from different files', () => { const mockDvcYamlContent = mockStagesLines.join('\n') const mockPlotYamlContent = [ '', diff --git a/extension/src/fileSystem/index.ts b/extension/src/fileSystem/index.ts index f6bf8e50c3..4e32be1f95 100644 --- a/extension/src/fileSystem/index.ts +++ b/extension/src/fileSystem/index.ts @@ -355,12 +355,12 @@ export const loadDataFiles = async ( const data = await loadDataFile(file) if (!data) { - continue + return undefined } filesData.push({ data, file }) } - return filesData.length === 0 ? undefined : filesData + return filesData } export const writeJson = < diff --git a/extension/src/pipeline/quickPick.test.ts b/extension/src/pipeline/quickPick.test.ts index 900a4c9cc7..d57cd5ef41 100644 --- a/extension/src/pipeline/quickPick.test.ts +++ b/extension/src/pipeline/quickPick.test.ts @@ -69,7 +69,7 @@ describe('pickPlotConfiguration', () => { expect(result).toStrictEqual(undefined) }) - it('should show a toast message if files(s) fail to parse', async () => { + it('should show a toast message if the file or files fail to parse', async () => { mockedPickFiles.mockResolvedValueOnce(['data.csv']) mockedLoadDataFiles.mockResolvedValueOnce(undefined) @@ -82,7 +82,7 @@ describe('pickPlotConfiguration', () => { ) }) - it('should show a toast message if an array of objects (with atleast two keys total) are not found within a file', async () => { + it('should show a toast message if an array of objects (with atleast two keys) are not found within a single chosen file', async () => { mockedPickFiles.mockResolvedValue(['file.yaml']) const invalidValues: unknown[] = [ 'string', @@ -133,16 +133,16 @@ describe('pickPlotConfiguration', () => { } }) - it('should show a toast message if atleast two fields are not found with multiple files', async () => { + it('should show a toast message if an array of objects (with atleast one key) are not found within multiple chosen files', async () => { mockedPickFiles.mockResolvedValueOnce([ 'file.yaml', 'file2.yaml', 'file3.yaml' ]) mockedLoadDataFiles.mockResolvedValueOnce([ - { data: '', file: 'file.yaml' }, - { data: { val: [{ field1: 'only one field' }] }, file: 'file2.yaml' }, - { data: [], file: 'file3.yaml' } + { data: { val: [{ field1: 1, field2: 2 }] }, file: 'file2.yaml' }, + { data: [], file: 'file3.yaml' }, + { data: { val: [{ field1: 1, field2: 2 }] }, file: 'file2.yaml' } ]) const result = await pickPlotConfiguration() @@ -183,36 +183,21 @@ describe('pickPlotConfiguration', () => { } }) - it('should parse fields from a mixture of valid and invalid files if atleast 2 fields are found in total', async () => { + it('should parse fields from multiple valid data files (if atleast two fields are found total)', async () => { mockedPickFiles.mockResolvedValueOnce([ 'file.yaml', 'file2.yaml', 'file3.yaml' ]) mockedLoadDataFiles.mockResolvedValueOnce([ - { - data: [ - { field1: 1, field2: 2, field3: 1, field4: 2 }, - { field1: 1, field2: 2, field3: 1, field4: 2 } - ], - file: 'file.yaml' - }, - { - data: { - field1: [{ field1: 1 }, { field1: 3 }] - }, - file: 'file3.yaml' - }, - { - data: undefined, - file: 'file3.yaml' - } + { data: { val: [{ field1: 1, field2: 2 }] }, file: 'file2.yaml' }, + { data: { val: [{ field1: 1 }] }, file: 'file2.yaml' } ]) - await pickPlotConfiguration() + const result = await pickPlotConfiguration() + expect(result).toStrictEqual(undefined) expect(mockedShowError).not.toHaveBeenCalled() - expect(mockedQuickPickOne).toHaveBeenCalledTimes(1) }) it('should let the user pick a template, x field, and y field', async () => { diff --git a/extension/src/pipeline/quickPick.ts b/extension/src/pipeline/quickPick.ts index c131003853..27172a1117 100644 --- a/extension/src/pipeline/quickPick.ts +++ b/extension/src/pipeline/quickPick.ts @@ -104,7 +104,7 @@ const getFieldOptionsFromArr = ( const fields = getFieldOptions(data) if (fields.length === 0) { - continue + return { keys: {}, keysAmount: 0 } } keysAmount += fields.length diff --git a/extension/src/vscode/resourcePicker.test.ts b/extension/src/vscode/resourcePicker.test.ts index 38ae071e40..bd3f1b8625 100644 --- a/extension/src/vscode/resourcePicker.test.ts +++ b/extension/src/vscode/resourcePicker.test.ts @@ -70,13 +70,15 @@ describe('pickResources', () => { describe('pickFiles', () => { it('should return an array of selected file paths', async () => { - const files = [resolve('mock', 'file'), resolve('mock', 'file2')] - const mockedPickedUris = files.map(file => Uri.file(file)) + const uris = [ + Uri.file(resolve('mock', 'file')), + Uri.file(resolve('mock', 'file2')) + ] const mockedTitle = 'insert great title here' as Title - mockedShowOpenDialog.mockResolvedValueOnce(mockedPickedUris) + mockedShowOpenDialog.mockResolvedValueOnce(uris) const pickedResources = await pickFiles(mockedTitle) - expect(pickedResources).toStrictEqual(files) + expect(pickedResources).toStrictEqual(uris.map(uri => uri.fsPath)) }) }) From 3d405fb43d760615e2a4f9e2c44bee3473c91c7f Mon Sep 17 00:00:00 2001 From: julieg18 Date: Tue, 3 Oct 2023 09:17:23 -0500 Subject: [PATCH 07/12] Improve error handling --- extension/src/pipeline/quickPick.test.ts | 10 +++++++ extension/src/pipeline/quickPick.ts | 37 +++++++++++++++++------- 2 files changed, 36 insertions(+), 11 deletions(-) diff --git a/extension/src/pipeline/quickPick.test.ts b/extension/src/pipeline/quickPick.test.ts index d57cd5ef41..c985c37c68 100644 --- a/extension/src/pipeline/quickPick.test.ts +++ b/extension/src/pipeline/quickPick.test.ts @@ -69,6 +69,16 @@ describe('pickPlotConfiguration', () => { expect(result).toStrictEqual(undefined) }) + it('should show a toast message if the files are not the same data type', async () => { + mockedPickFiles.mockResolvedValueOnce(['file.json', 'file.csv']) + + const result = await pickPlotConfiguration() + + expect(result).toStrictEqual(undefined) + expect(mockedShowError).toHaveBeenCalledTimes(1) + expect(mockedShowError).toHaveBeenCalledWith('Files must of the same type.') + }) + it('should show a toast message if the file or files fail to parse', async () => { mockedPickFiles.mockResolvedValueOnce(['data.csv']) mockedLoadDataFiles.mockResolvedValueOnce(undefined) diff --git a/extension/src/pipeline/quickPick.ts b/extension/src/pipeline/quickPick.ts index 27172a1117..e64f8d921e 100644 --- a/extension/src/pipeline/quickPick.ts +++ b/extension/src/pipeline/quickPick.ts @@ -6,7 +6,7 @@ import { ValueTree, isValueTree } from '../cli/dvc/contract' -import { loadDataFiles } from '../fileSystem' +import { getFileExtension, loadDataFiles } from '../fileSystem' import { quickPickOne, quickPickValue } from '../vscode/quickPick' import { pickFiles } from '../vscode/resourcePicker' import { Title } from '../vscode/title' @@ -78,7 +78,7 @@ const getFieldsFromArr = (dataArr: UnknownValue[]) => { return objsHaveSameKeys ? fieldObjKeys : [] } -const getFieldOptions = (data: UnknownValue): string[] => { +const getFieldsFromValue = (data: UnknownValue): string[] => { const isArray = Array.isArray(data) const isObj = isValueTree(data) if (!isArray && !isObj) { @@ -92,7 +92,12 @@ const getFieldOptions = (data: UnknownValue): string[] => { : [] } -const getFieldOptionsFromArr = ( +const showNotEnoughKeysToast = () => + Toast.showError( + 'The requested file(s) does not contain enough keys (columns) to generate a plot. Does the file or files follow the DVC plot guidelines for [JSON/YAML](https://dvc.org/doc/command-reference/plots/show#example-hierarchical-data) or [CSV/TSV](https://dvc.org/doc/command-reference/plots/show#example-tabular-data) files?' + ) + +const getFieldsFromDataFiles = ( dataArr: { data: UnknownValue; file: string }[] ) => { const keys: { @@ -101,10 +106,11 @@ const getFieldOptionsFromArr = ( let keysAmount = 0 for (const { file, data } of dataArr) { - const fields = getFieldOptions(data) + const fields = getFieldsFromValue(data) if (fields.length === 0) { - return { keys: {}, keysAmount: 0 } + void showNotEnoughKeysToast() + return } keysAmount += fields.length @@ -115,7 +121,12 @@ const getFieldOptionsFromArr = ( keys[file].push(...fields) } - return { keys, keysAmount } + if (keysAmount < 2) { + void showNotEnoughKeysToast() + return + } + + return keys } export const pickPlotConfiguration = async (): Promise< @@ -127,6 +138,12 @@ export const pickPlotConfiguration = async (): Promise< return } + const fileExts = new Set(files.map(file => getFileExtension(file))) + + if (fileExts.size > 1) { + return Toast.showError('Files must of the same type.') + } + const filesData = await loadDataFiles(files) if (!filesData) { @@ -135,14 +152,12 @@ export const pickPlotConfiguration = async (): Promise< ) } - const { keys, keysAmount } = getFieldOptionsFromArr( + const keys = getFieldsFromDataFiles( filesData as { data: UnknownValue; file: string }[] ) - if (keysAmount < 2) { - return Toast.showError( - 'The requested file(s) does not contain enough keys (columns) to generate a plot. Does the file or files follow the DVC plot guidelines for [JSON/YAML](https://dvc.org/doc/command-reference/plots/show#example-hierarchical-data) or [CSV/TSV](https://dvc.org/doc/command-reference/plots/show#example-tabular-data) files?' - ) + if (!keys) { + return } const templateAndFields = await pickTemplateAndFields(keys) From 76df14f39d972fa6cb05393ea0c71e1dada4ab17 Mon Sep 17 00:00:00 2001 From: julieg18 Date: Tue, 3 Oct 2023 09:37:46 -0500 Subject: [PATCH 08/12] Fix broken pickFiles --- extension/src/vscode/resourcePicker.test.ts | 30 +++++++++++++++----- extension/src/vscode/resourcePicker.ts | 31 ++++++++++++--------- 2 files changed, 41 insertions(+), 20 deletions(-) diff --git a/extension/src/vscode/resourcePicker.test.ts b/extension/src/vscode/resourcePicker.test.ts index bd3f1b8625..b6b61ce2b1 100644 --- a/extension/src/vscode/resourcePicker.test.ts +++ b/extension/src/vscode/resourcePicker.test.ts @@ -69,16 +69,32 @@ describe('pickResources', () => { }) describe('pickFiles', () => { - it('should return an array of selected file paths', async () => { - const uris = [ - Uri.file(resolve('mock', 'file')), - Uri.file(resolve('mock', 'file2')) + it('should call window.showOpenDialog with the correct options', async () => { + const mockedTitle = 'I decided to not decide' as Title + mockedShowOpenDialog.mockResolvedValueOnce(undefined) + + await pickFiles(mockedTitle, { Images: ['png', 'jpg'] }) + + expect(mockedShowOpenDialog).toHaveBeenCalledWith({ + canSelectFiles: true, + canSelectFolders: false, + canSelectMany: true, + filters: { Images: ['png', 'jpg'] }, + openLabel: 'Select', + title: mockedTitle + }) + }) + + it('should return an array of paths if any are selected', async () => { + const mockedUris = [ + Uri.file(resolve('mock', 'file1.json')), + Uri.file(resolve('mock', 'file2.json')) ] - const mockedTitle = 'insert great title here' as Title - mockedShowOpenDialog.mockResolvedValueOnce(uris) + const mockedTitle = 'this is even more fun' as Title + mockedShowOpenDialog.mockResolvedValueOnce(mockedUris) const pickedResources = await pickFiles(mockedTitle) - expect(pickedResources).toStrictEqual(uris.map(uri => uri.fsPath)) + expect(pickedResources).toStrictEqual(mockedUris.map(uri => uri.fsPath)) }) }) diff --git a/extension/src/vscode/resourcePicker.ts b/extension/src/vscode/resourcePicker.ts index 651a6b98f7..d5fc137505 100644 --- a/extension/src/vscode/resourcePicker.ts +++ b/extension/src/vscode/resourcePicker.ts @@ -19,30 +19,35 @@ export const pickFile = async ( } } -export const pickResources = ( +export const pickResources = (title: Title): Thenable => { + return window.showOpenDialog({ + canSelectFiles: true, + canSelectFolders: true, + canSelectMany: true, + openLabel: 'Select', + title + }) +} + +export const pickFiles = async ( title: Title, filters?: OpenDialogOptions['filters'] -): Thenable => { +): Promise => { const opts: OpenDialogOptions = { canSelectFiles: true, - canSelectFolders: true, + canSelectFolders: false, canSelectMany: true, openLabel: 'Select', title } - if (!filters) { + if (filters) { opts.filters = filters } - return window.showOpenDialog(opts) -} - -export const pickFiles = async ( - title: Title, - filters?: OpenDialogOptions['filters'] -): Promise => { - const files = await pickResources(title, filters) + const uris = await window.showOpenDialog(opts) - return files?.map(({ fsPath }) => fsPath) + if (uris) { + return uris.map(({ fsPath }) => fsPath) + } } From 22c64c50477df429e888f33e48b21e861d2e26a2 Mon Sep 17 00:00:00 2001 From: julieg18 Date: Tue, 3 Oct 2023 10:39:14 -0500 Subject: [PATCH 09/12] Use relative path instead of just filename in quick pick --- extension/src/pipeline/index.ts | 2 +- extension/src/pipeline/quickPick.test.ts | 72 ++++++++++++------------ extension/src/pipeline/quickPick.ts | 21 ++++--- 3 files changed, 49 insertions(+), 46 deletions(-) diff --git a/extension/src/pipeline/index.ts b/extension/src/pipeline/index.ts index d69f0944f8..ae019ad728 100644 --- a/extension/src/pipeline/index.ts +++ b/extension/src/pipeline/index.ts @@ -119,7 +119,7 @@ export class Pipeline extends DeferredDisposable { return } - const plotConfiguration = await pickPlotConfiguration() + const plotConfiguration = await pickPlotConfiguration(cwd) if (!plotConfiguration) { return diff --git a/extension/src/pipeline/quickPick.test.ts b/extension/src/pipeline/quickPick.test.ts index c985c37c68..0c08efe5ee 100644 --- a/extension/src/pipeline/quickPick.test.ts +++ b/extension/src/pipeline/quickPick.test.ts @@ -54,7 +54,7 @@ describe('pickPlotConfiguration', () => { { data: mockValidData, file: 'file.json' } ]) - await pickPlotConfiguration() + await pickPlotConfiguration('/') expect(mockedPickFiles).toHaveBeenCalledWith(Title.SELECT_PLOT_DATA, { 'Data Formats': ['json', 'csv', 'tsv', 'yaml'] @@ -64,7 +64,7 @@ describe('pickPlotConfiguration', () => { it('should return early if user does not select a file', async () => { mockedPickFiles.mockResolvedValueOnce(undefined) - const result = await pickPlotConfiguration() + const result = await pickPlotConfiguration('/') expect(result).toStrictEqual(undefined) }) @@ -72,7 +72,7 @@ describe('pickPlotConfiguration', () => { it('should show a toast message if the files are not the same data type', async () => { mockedPickFiles.mockResolvedValueOnce(['file.json', 'file.csv']) - const result = await pickPlotConfiguration() + const result = await pickPlotConfiguration('/') expect(result).toStrictEqual(undefined) expect(mockedShowError).toHaveBeenCalledTimes(1) @@ -83,7 +83,7 @@ describe('pickPlotConfiguration', () => { mockedPickFiles.mockResolvedValueOnce(['data.csv']) mockedLoadDataFiles.mockResolvedValueOnce(undefined) - const result = await pickPlotConfiguration() + const result = await pickPlotConfiguration('/') expect(result).toStrictEqual(undefined) expect(mockedShowError).toHaveBeenCalledTimes(1) @@ -133,7 +133,7 @@ describe('pickPlotConfiguration', () => { { data: invalidVal, file: 'file.yaml' } ]) - result = await pickPlotConfiguration() + result = await pickPlotConfiguration('/') expect(result).toStrictEqual(undefined) expect(mockedShowError).toHaveBeenCalledTimes(1 + ind) @@ -155,7 +155,7 @@ describe('pickPlotConfiguration', () => { { data: { val: [{ field1: 1, field2: 2 }] }, file: 'file2.yaml' } ]) - const result = await pickPlotConfiguration() + const result = await pickPlotConfiguration('/') expect(result).toStrictEqual(undefined) expect(mockedShowError).toHaveBeenCalledTimes(1) @@ -186,7 +186,7 @@ describe('pickPlotConfiguration', () => { { data: val, file: 'file.yaml' } ]) - await pickPlotConfiguration() + await pickPlotConfiguration('/') expect(mockedShowError).not.toHaveBeenCalled() expect(mockedQuickPickOne).toHaveBeenCalledTimes(ind + 1) @@ -204,23 +204,23 @@ describe('pickPlotConfiguration', () => { { data: { val: [{ field1: 1 }] }, file: 'file2.yaml' } ]) - const result = await pickPlotConfiguration() + const result = await pickPlotConfiguration('/') expect(result).toStrictEqual(undefined) expect(mockedShowError).not.toHaveBeenCalled() }) it('should let the user pick a template, x field, and y field', async () => { - mockedPickFiles.mockResolvedValueOnce(['file.json']) + mockedPickFiles.mockResolvedValueOnce(['/file.json']) mockedLoadDataFiles.mockResolvedValueOnce([ - { data: mockValidData, file: 'file.json' } + { data: mockValidData, file: '/file.json' } ]) mockedQuickPickOne.mockResolvedValueOnce('simple') mockedQuickPickValue - .mockResolvedValueOnce({ file: 'file.json', key: 'actual' }) - .mockResolvedValueOnce({ file: 'file.json', key: 'prob' }) + .mockResolvedValueOnce({ file: '/file.json', key: 'actual' }) + .mockResolvedValueOnce({ file: '/file.json', key: 'prob' }) - const result = await pickPlotConfiguration() + const result = await pickPlotConfiguration('/') expect(mockedQuickPickOne).toHaveBeenNthCalledWith( 1, @@ -245,8 +245,8 @@ describe('pickPlotConfiguration', () => { label: 'file.json', value: undefined }, - { label: 'actual', value: { file: 'file.json', key: 'actual' } }, - { label: 'prob', value: { file: 'file.json', key: 'prob' } } + { label: 'actual', value: { file: '/file.json', key: 'actual' } }, + { label: 'prob', value: { file: '/file.json', key: 'prob' } } ], { title: Title.SELECT_PLOT_X_METRIC } ) @@ -258,7 +258,7 @@ describe('pickPlotConfiguration', () => { label: 'file.json', value: undefined }, - { label: 'prob', value: { file: 'file.json', key: 'prob' } } + { label: 'prob', value: { file: '/file.json', key: 'prob' } } ], { title: Title.SELECT_PLOT_Y_METRIC @@ -266,23 +266,23 @@ describe('pickPlotConfiguration', () => { ) expect(result).toStrictEqual({ template: 'simple', - x: { file: 'file.json', key: 'actual' }, - y: { file: 'file.json', key: 'prob' } + x: { file: '/file.json', key: 'actual' }, + y: { file: '/file.json', key: 'prob' } }) }) it('should let the user pick a x field and y field from multiple files', async () => { - mockedPickFiles.mockResolvedValueOnce(['file.json', 'file2.json']) + mockedPickFiles.mockResolvedValueOnce(['/file.json', '/file2.json']) mockedLoadDataFiles.mockResolvedValueOnce([ - { data: mockValidData, file: 'file.json' }, - { data: mockValidData, file: 'file2.json' } + { data: mockValidData, file: '/file.json' }, + { data: mockValidData, file: '/file2.json' } ]) mockedQuickPickOne.mockResolvedValueOnce('simple') mockedQuickPickValue - .mockResolvedValueOnce({ file: 'file.json', key: 'actual' }) - .mockResolvedValueOnce({ file: 'file2.json', key: 'prob' }) + .mockResolvedValueOnce({ file: '/file.json', key: 'actual' }) + .mockResolvedValueOnce({ file: '/file2.json', key: 'prob' }) - const result = await pickPlotConfiguration() + const result = await pickPlotConfiguration('/') expect(mockedQuickPickValue).toHaveBeenNthCalledWith( 1, @@ -292,15 +292,15 @@ describe('pickPlotConfiguration', () => { label: 'file.json', value: undefined }, - { label: 'actual', value: { file: 'file.json', key: 'actual' } }, - { label: 'prob', value: { file: 'file.json', key: 'prob' } }, + { label: 'actual', value: { file: '/file.json', key: 'actual' } }, + { label: 'prob', value: { file: '/file.json', key: 'prob' } }, { kind: QuickPickItemKind.Separator, label: 'file2.json', value: undefined }, - { label: 'actual', value: { file: 'file2.json', key: 'actual' } }, - { label: 'prob', value: { file: 'file2.json', key: 'prob' } } + { label: 'actual', value: { file: '/file2.json', key: 'actual' } }, + { label: 'prob', value: { file: '/file2.json', key: 'prob' } } ], { title: Title.SELECT_PLOT_X_METRIC } ) @@ -312,14 +312,14 @@ describe('pickPlotConfiguration', () => { label: 'file.json', value: undefined }, - { label: 'prob', value: { file: 'file.json', key: 'prob' } }, + { label: 'prob', value: { file: '/file.json', key: 'prob' } }, { kind: QuickPickItemKind.Separator, label: 'file2.json', value: undefined }, - { label: 'actual', value: { file: 'file2.json', key: 'actual' } }, - { label: 'prob', value: { file: 'file2.json', key: 'prob' } } + { label: 'actual', value: { file: '/file2.json', key: 'actual' } }, + { label: 'prob', value: { file: '/file2.json', key: 'prob' } } ], { title: Title.SELECT_PLOT_Y_METRIC @@ -327,8 +327,8 @@ describe('pickPlotConfiguration', () => { ) expect(result).toStrictEqual({ template: 'simple', - x: { file: 'file.json', key: 'actual' }, - y: { file: 'file2.json', key: 'prob' } + x: { file: '/file.json', key: 'actual' }, + y: { file: '/file2.json', key: 'prob' } }) }) @@ -339,7 +339,7 @@ describe('pickPlotConfiguration', () => { ]) mockedQuickPickOne.mockResolvedValueOnce(undefined) - const result = await pickPlotConfiguration() + const result = await pickPlotConfiguration('/') expect(mockedQuickPickOne).toHaveBeenCalledTimes(1) @@ -354,7 +354,7 @@ describe('pickPlotConfiguration', () => { mockedQuickPickOne.mockResolvedValueOnce('simple') mockedQuickPickValue.mockResolvedValueOnce(undefined) - const result = await pickPlotConfiguration() + const result = await pickPlotConfiguration('/') expect(mockedQuickPickValue).toHaveBeenCalledTimes(1) expect(result).toStrictEqual(undefined) @@ -370,7 +370,7 @@ describe('pickPlotConfiguration', () => { .mockResolvedValueOnce('actual') .mockResolvedValueOnce(undefined) - const result = await pickPlotConfiguration() + const result = await pickPlotConfiguration('/') expect(mockedQuickPickValue).toHaveBeenCalledTimes(2) expect(result).toStrictEqual(undefined) diff --git a/extension/src/pipeline/quickPick.ts b/extension/src/pipeline/quickPick.ts index e64f8d921e..34a89d2fc7 100644 --- a/extension/src/pipeline/quickPick.ts +++ b/extension/src/pipeline/quickPick.ts @@ -1,3 +1,4 @@ +import { relative } from 'path' import isEqual from 'lodash.isequal' import { QuickPickItemKind } from 'vscode' import { @@ -11,16 +12,18 @@ import { quickPickOne, quickPickValue } from '../vscode/quickPick' import { pickFiles } from '../vscode/resourcePicker' import { Title } from '../vscode/title' import { Toast } from '../vscode/toast' -import { getFileName } from '../fileSystem/util' const pickDataFiles = (): Thenable => pickFiles(Title.SELECT_PLOT_DATA, { 'Data Formats': ['json', 'csv', 'tsv', 'yaml'] }) -const pickTemplateAndFields = async (fields: { - [file: string]: string[] -}): Promise => { +const pickTemplateAndFields = async ( + cwd: string, + fields: { + [file: string]: string[] + } +): Promise => { const template = await quickPickOne(PLOT_TEMPLATES, 'Pick a Plot Template') if (!template) { @@ -33,7 +36,7 @@ const pickTemplateAndFields = async (fields: { items.push( { kind: QuickPickItemKind.Separator, - label: getFileName(file), + label: relative(cwd, file), value: undefined }, ...keys.map(key => ({ label: key, value: { file, key } })) @@ -129,9 +132,9 @@ const getFieldsFromDataFiles = ( return keys } -export const pickPlotConfiguration = async (): Promise< - PlotConfigData | undefined -> => { +export const pickPlotConfiguration = async ( + cwd: string +): Promise => { const files = await pickDataFiles() if (!files) { @@ -160,7 +163,7 @@ export const pickPlotConfiguration = async (): Promise< return } - const templateAndFields = await pickTemplateAndFields(keys) + const templateAndFields = await pickTemplateAndFields(cwd, keys) if (!templateAndFields) { return From da06e65a0273232caeafa99e3bdf14be1f6240f0 Mon Sep 17 00:00:00 2001 From: julieg18 Date: Tue, 3 Oct 2023 11:05:26 -0500 Subject: [PATCH 10/12] Delete unneeded function --- extension/src/fileSystem/util.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/extension/src/fileSystem/util.ts b/extension/src/fileSystem/util.ts index 53effd7ac0..36ab159925 100644 --- a/extension/src/fileSystem/util.ts +++ b/extension/src/fileSystem/util.ts @@ -22,6 +22,4 @@ export const getParent = (pathArray: string[], idx: number) => { export const removeTrailingSlash = (path: string): string => path.endsWith(sep) ? path.slice(0, -1) : path -export const getFileName = (path: string) => parse(path).base - export const getFileNameWithoutExt = (path: string) => parse(path).name From b6af87c6aa4a837b9be0d3ecf9e3a706fc35d279 Mon Sep 17 00:00:00 2001 From: julieg18 Date: Wed, 4 Oct 2023 08:36:07 -0500 Subject: [PATCH 11/12] Resolve some comments: * simplify plot object * simplify resourcePicker --- extension/src/fileSystem/index.test.ts | 10 +++-- extension/src/fileSystem/index.ts | 9 +--- extension/src/vscode/resourcePicker.test.ts | 1 + extension/src/vscode/resourcePicker.ts | 49 ++++++++------------- 4 files changed, 27 insertions(+), 42 deletions(-) diff --git a/extension/src/fileSystem/index.test.ts b/extension/src/fileSystem/index.test.ts index 90fa5be5ee..65a5874bc4 100644 --- a/extension/src/fileSystem/index.test.ts +++ b/extension/src/fileSystem/index.test.ts @@ -592,10 +592,11 @@ describe('addPlotToDvcYamlFile', () => { ' eval/prc/test.json: precision' ] const mockNewPlotLines = [ - ' - data.json:', + ' - simple_plot:', ' template: simple', ' x: epochs', - ' y: accuracy' + ' y:', + ' data.json: accuracy' ] it('should add a plots list with the new plot if the dvc.yaml file has no plots', () => { const mockDvcYamlContent = mockStagesLines.join('\n') @@ -700,10 +701,11 @@ describe('addPlotToDvcYamlFile', () => { ].join('\n') const mockPlotYamlContent = [ '', - ' - data.json:', + ' - simple_plot:', ' template: simple', ' x: epochs', - ' y: accuracy', + ' y:', + ' data.json: accuracy', '' ].join('\n') mockedReadFileSync.mockReturnValueOnce(mockDvcYamlContent) diff --git a/extension/src/fileSystem/index.ts b/extension/src/fileSystem/index.ts index 4e32be1f95..f79dcb6211 100644 --- a/extension/src/fileSystem/index.ts +++ b/extension/src/fileSystem/index.ts @@ -216,18 +216,11 @@ const loadYamlAsDoc = ( const getPlotYamlObj = (cwd: string, plot: PlotConfigData) => { const { x, y, template } = plot - const usesSingleFile = x.file === y.file - if (usesSingleFile) { - const dataFile = x.file - const plotName = relative(cwd, dataFile) - return { [plotName]: { template, x: x.key, y: y.key } } - } - const plotName = `${template}_plot` return { [plotName]: { template, - x: { [relative(cwd, x.file)]: x.key }, + x: x.file === y.file ? x.key : { [relative(cwd, x.file)]: x.key }, y: { [relative(cwd, y.file)]: y.key } } } diff --git a/extension/src/vscode/resourcePicker.test.ts b/extension/src/vscode/resourcePicker.test.ts index b6b61ce2b1..69050eb2dc 100644 --- a/extension/src/vscode/resourcePicker.test.ts +++ b/extension/src/vscode/resourcePicker.test.ts @@ -21,6 +21,7 @@ describe('pickFile', () => { await pickFile(mockedTitle) expect(mockedShowOpenDialog).toHaveBeenCalledWith({ + canSelectFiles: true, canSelectFolders: false, canSelectMany: false, openLabel: 'Select', diff --git a/extension/src/vscode/resourcePicker.ts b/extension/src/vscode/resourcePicker.ts index d5fc137505..87991ee764 100644 --- a/extension/src/vscode/resourcePicker.ts +++ b/extension/src/vscode/resourcePicker.ts @@ -1,51 +1,40 @@ import { OpenDialogOptions, Uri, window } from 'vscode' import { Title } from './title' -export const pickFile = async ( +export const pickResources = ( title: Title, + canSelectFolders = true, + canSelectMany = true, filters?: OpenDialogOptions['filters'] -): Promise => { - const uris = await window.showOpenDialog({ - canSelectFolders: false, - canSelectMany: false, - filters, +): Thenable => { + const opts: OpenDialogOptions = { + canSelectFiles: true, + canSelectFolders, + canSelectMany, openLabel: 'Select', title - }) + } + + if (filters) { + opts.filters = filters + } + + return window.showOpenDialog(opts) +} +export const pickFile = async (title: Title): Promise => { + const uris = await pickResources(title, false, false) if (uris) { const [{ fsPath }] = uris return fsPath } } -export const pickResources = (title: Title): Thenable => { - return window.showOpenDialog({ - canSelectFiles: true, - canSelectFolders: true, - canSelectMany: true, - openLabel: 'Select', - title - }) -} - export const pickFiles = async ( title: Title, filters?: OpenDialogOptions['filters'] ): Promise => { - const opts: OpenDialogOptions = { - canSelectFiles: true, - canSelectFolders: false, - canSelectMany: true, - openLabel: 'Select', - title - } - - if (filters) { - opts.filters = filters - } - - const uris = await window.showOpenDialog(opts) + const uris = await pickResources(title, false, true, filters) if (uris) { return uris.map(({ fsPath }) => fsPath) From bd3e211c441480c7c620df19d4b03d13ed039c32 Mon Sep 17 00:00:00 2001 From: julieg18 Date: Wed, 4 Oct 2023 09:25:02 -0500 Subject: [PATCH 12/12] Improve text --- extension/src/pipeline/quickPick.test.ts | 6 +++--- extension/src/pipeline/quickPick.ts | 4 ++-- extension/src/plots/quickPick.test.ts | 2 +- extension/src/plots/quickPick.ts | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/extension/src/pipeline/quickPick.test.ts b/extension/src/pipeline/quickPick.test.ts index 0c08efe5ee..05bf97bfd2 100644 --- a/extension/src/pipeline/quickPick.test.ts +++ b/extension/src/pipeline/quickPick.test.ts @@ -88,7 +88,7 @@ describe('pickPlotConfiguration', () => { expect(result).toStrictEqual(undefined) expect(mockedShowError).toHaveBeenCalledTimes(1) expect(mockedShowError).toHaveBeenCalledWith( - 'Failed to parse the requested file(s). Does the file or files contain data and follow the DVC plot guidelines for [JSON/YAML](https://dvc.org/doc/command-reference/plots/show#example-hierarchical-data) or [CSV/TSV](https://dvc.org/doc/command-reference/plots/show#example-tabular-data) files?' + 'Failed to parse the requested file or files. Does the file or files contain data and follow the DVC plot guidelines for [JSON/YAML](https://dvc.org/doc/command-reference/plots/show#example-hierarchical-data) or [CSV/TSV](https://dvc.org/doc/command-reference/plots/show#example-tabular-data) files?' ) }) @@ -138,7 +138,7 @@ describe('pickPlotConfiguration', () => { expect(result).toStrictEqual(undefined) expect(mockedShowError).toHaveBeenCalledTimes(1 + ind) expect(mockedShowError).toHaveBeenCalledWith( - 'The requested file(s) does not contain enough keys (columns) to generate a plot. Does the file or files follow the DVC plot guidelines for [JSON/YAML](https://dvc.org/doc/command-reference/plots/show#example-hierarchical-data) or [CSV/TSV](https://dvc.org/doc/command-reference/plots/show#example-tabular-data) files?' + 'The requested file or files do not contain enough keys (columns) to generate a plot. Does the file or files follow the DVC plot guidelines for [JSON/YAML](https://dvc.org/doc/command-reference/plots/show#example-hierarchical-data) or [CSV/TSV](https://dvc.org/doc/command-reference/plots/show#example-tabular-data) files?' ) } }) @@ -160,7 +160,7 @@ describe('pickPlotConfiguration', () => { expect(result).toStrictEqual(undefined) expect(mockedShowError).toHaveBeenCalledTimes(1) expect(mockedShowError).toHaveBeenCalledWith( - 'The requested file(s) does not contain enough keys (columns) to generate a plot. Does the file or files follow the DVC plot guidelines for [JSON/YAML](https://dvc.org/doc/command-reference/plots/show#example-hierarchical-data) or [CSV/TSV](https://dvc.org/doc/command-reference/plots/show#example-tabular-data) files?' + 'The requested file or files do not contain enough keys (columns) to generate a plot. Does the file or files follow the DVC plot guidelines for [JSON/YAML](https://dvc.org/doc/command-reference/plots/show#example-hierarchical-data) or [CSV/TSV](https://dvc.org/doc/command-reference/plots/show#example-tabular-data) files?' ) }) diff --git a/extension/src/pipeline/quickPick.ts b/extension/src/pipeline/quickPick.ts index 34a89d2fc7..0a2d994431 100644 --- a/extension/src/pipeline/quickPick.ts +++ b/extension/src/pipeline/quickPick.ts @@ -97,7 +97,7 @@ const getFieldsFromValue = (data: UnknownValue): string[] => { const showNotEnoughKeysToast = () => Toast.showError( - 'The requested file(s) does not contain enough keys (columns) to generate a plot. Does the file or files follow the DVC plot guidelines for [JSON/YAML](https://dvc.org/doc/command-reference/plots/show#example-hierarchical-data) or [CSV/TSV](https://dvc.org/doc/command-reference/plots/show#example-tabular-data) files?' + 'The requested file or files do not contain enough keys (columns) to generate a plot. Does the file or files follow the DVC plot guidelines for [JSON/YAML](https://dvc.org/doc/command-reference/plots/show#example-hierarchical-data) or [CSV/TSV](https://dvc.org/doc/command-reference/plots/show#example-tabular-data) files?' ) const getFieldsFromDataFiles = ( @@ -151,7 +151,7 @@ export const pickPlotConfiguration = async ( if (!filesData) { return Toast.showError( - 'Failed to parse the requested file(s). Does the file or files contain data and follow the DVC plot guidelines for [JSON/YAML](https://dvc.org/doc/command-reference/plots/show#example-hierarchical-data) or [CSV/TSV](https://dvc.org/doc/command-reference/plots/show#example-tabular-data) files?' + 'Failed to parse the requested file or files. Does the file or files contain data and follow the DVC plot guidelines for [JSON/YAML](https://dvc.org/doc/command-reference/plots/show#example-hierarchical-data) or [CSV/TSV](https://dvc.org/doc/command-reference/plots/show#example-tabular-data) files?' ) } diff --git a/extension/src/plots/quickPick.test.ts b/extension/src/plots/quickPick.test.ts index 7e22b95dee..8bcaa48235 100644 --- a/extension/src/plots/quickPick.test.ts +++ b/extension/src/plots/quickPick.test.ts @@ -20,7 +20,7 @@ describe('pickPlotType', () => { [ { description: - 'Create a data series plot definition by selecting data from a file', + 'Create a data series plot definition by selecting data from one or more files (To select multiple files, hold the Ctrl/Command key and click on the files.).', label: 'Data Series', value: 'data-series' }, diff --git a/extension/src/plots/quickPick.ts b/extension/src/plots/quickPick.ts index 5c00508247..e2358bb262 100644 --- a/extension/src/plots/quickPick.ts +++ b/extension/src/plots/quickPick.ts @@ -11,7 +11,7 @@ export const pickPlotType = () => [ { description: - 'Create a data series plot definition by selecting data from a file', + 'Create a data series plot definition by selecting data from one or more files (To select multiple files, hold the Ctrl/Command key and click on the files.).', label: 'Data Series', value: PLOT_TYPE.DATA_SERIES },