diff --git a/extension/package.json b/extension/package.json index aeec7b5b11..1415621b2a 100644 --- a/extension/package.json +++ b/extension/package.json @@ -1688,7 +1688,7 @@ "fork-ts-checker-webpack-plugin": "8.0.0", "jest": "29.5.0", "jest-environment-node": "29.5.0", - "lint-staged": "13.2.0", + "lint-staged": "13.2.1", "mocha": "10.2.0", "mock-require": "3.0.3", "process-exists": "4.1.0", diff --git a/extension/src/experiments/workspace.test.ts b/extension/src/experiments/workspace.test.ts index db5336876c..da2abbe3f8 100644 --- a/extension/src/experiments/workspace.test.ts +++ b/extension/src/experiments/workspace.test.ts @@ -19,6 +19,7 @@ import { hasDvcYamlFile } from '../fileSystem' import { Toast } from '../vscode/toast' +import { pickFile } from '../vscode/resourcePicker' const mockedShowWebview = jest.fn() const mockedDisposable = jest.mocked(Disposable) @@ -35,12 +36,14 @@ const mockedListStages = jest.fn() const mockedFindOrCreateDvcYamlFile = jest.mocked(findOrCreateDvcYamlFile) const mockedGetFileExtension = jest.mocked(getFileExtension) const mockedHasDvcYamlFile = jest.mocked(hasDvcYamlFile) +const mockedPickFile = jest.mocked(pickFile) jest.mock('vscode') jest.mock('@hediet/std/disposable') jest.mock('../vscode/quickPick') jest.mock('../vscode/inputBox') jest.mock('../fileSystem') +jest.mock('../vscode/resourcePicker') beforeEach(() => { jest.resetAllMocks() @@ -442,7 +445,8 @@ describe('Experiments', () => { mockedDvcRoot, trainingScript, 'train', - scriptCommand.PYTHON + scriptCommand.PYTHON, + false ) }) @@ -482,7 +486,8 @@ describe('Experiments', () => { mockedDvcRoot, 'path/to/training_script.py', 'train', - scriptCommand.PYTHON + scriptCommand.PYTHON, + false ) }) @@ -501,7 +506,8 @@ describe('Experiments', () => { mockedDvcRoot, 'path/to/training_script.ipynb', 'train', - scriptCommand.JUPYTER + scriptCommand.JUPYTER, + false ) }) @@ -553,7 +559,53 @@ describe('Experiments', () => { mockedDvcRoot, 'path/to/training_script.js', 'train', - customCommand + customCommand, + false + ) + }) + + it('should not convert the script path to relative if the path was entered manually', async () => { + const customCommand = 'node' + + mockedGetValidInput.mockResolvedValueOnce('train') + mockedListStages.mockResolvedValueOnce('') + mockedQuickPickOne.mockResolvedValueOnce(mockedDvcRoot) + mockedQuickPickOneOrInput.mockResolvedValueOnce( + 'path/to/training_script.js' + ) + mockedGetFileExtension.mockReturnValueOnce('.js') + mockedGetInput.mockResolvedValueOnce(customCommand) + + await workspaceExperiments.getCwdThenRun(mockedCommandId) + + expect(mockedFindOrCreateDvcYamlFile).toHaveBeenCalledWith( + mockedDvcRoot, + 'path/to/training_script.js', + 'train', + customCommand, + false + ) + }) + + it('should convert the script path to relative if the path was not entered manually', async () => { + const customCommand = 'node' + + mockedGetValidInput.mockResolvedValueOnce('train') + mockedListStages.mockResolvedValueOnce('') + mockedQuickPickOne.mockResolvedValueOnce(mockedDvcRoot) + mockedQuickPickOneOrInput.mockResolvedValueOnce('select') + mockedPickFile.mockResolvedValueOnce('path/to/training_script.js') + mockedGetFileExtension.mockReturnValueOnce('.js') + mockedGetInput.mockResolvedValueOnce(customCommand) + + await workspaceExperiments.getCwdThenRun(mockedCommandId) + + expect(mockedFindOrCreateDvcYamlFile).toHaveBeenCalledWith( + mockedDvcRoot, + 'path/to/training_script.js', + 'train', + customCommand, + true ) }) @@ -573,7 +625,8 @@ describe('Experiments', () => { mockedDvcRoot, 'path/to/training_script.js', 'train', - '' + '', + false ) }) diff --git a/extension/src/experiments/workspace.ts b/extension/src/experiments/workspace.ts index 17af401988..85a1965354 100644 --- a/extension/src/experiments/workspace.ts +++ b/extension/src/experiments/workspace.ts @@ -452,11 +452,18 @@ export class WorkspaceExperiments extends BaseWorkspaceWebviews< return false } - const { trainingScript, command } = await this.askForTrainingScript() + const { trainingScript, command, enteredManually } = + await this.askForTrainingScript() if (!trainingScript) { return false } - void findOrCreateDvcYamlFile(cwd, trainingScript, stageName, command) + void findOrCreateDvcYamlFile( + cwd, + trainingScript, + stageName, + command, + !enteredManually + ) return true } @@ -495,14 +502,19 @@ export class WorkspaceExperiments extends BaseWorkspaceWebviews< : pathOrSelect if (!trainingScript) { - return { command: undefined, trainingScript: undefined } + return { + command: undefined, + enteredManually: false, + trainingScript: undefined + } } const command = getScriptCommand(trainingScript) || (await getInput(Title.ENTER_COMMAND_TO_RUN)) || '' - return { command, trainingScript } + const enteredManually = pathOrSelect !== selectValue + return { command, enteredManually, trainingScript } } private async pickExpThenRun( diff --git a/extension/src/fileSystem/index.test.ts b/extension/src/fileSystem/index.test.ts index e128568fca..55c4d58544 100644 --- a/extension/src/fileSystem/index.test.ts +++ b/extension/src/fileSystem/index.test.ts @@ -187,7 +187,8 @@ describe('findOrCreateDvcYamlFile', () => { cwd, '/my/training/script.py', 'train', - scriptCommand.PYTHON + scriptCommand.PYTHON, + true ) expect(mockedEnsureFileSync).toHaveBeenCalledWith(`${cwd}/dvc.yaml`) @@ -200,7 +201,8 @@ describe('findOrCreateDvcYamlFile', () => { cwd, '/script.py', uniqueStageName, - scriptCommand.PYTHON + scriptCommand.PYTHON, + true ) expect(mockedAppendFileSync).toHaveBeenCalledWith( @@ -215,7 +217,8 @@ describe('findOrCreateDvcYamlFile', () => { cwd, '/my/training/script.py', 'train', - scriptCommand.PYTHON + scriptCommand.PYTHON, + true ) expect(mockedAppendFileSync).toHaveBeenCalledWith( @@ -230,7 +233,8 @@ describe('findOrCreateDvcYamlFile', () => { cwd, '/my/training/script.py', 'train', - scriptCommand.PYTHON + scriptCommand.PYTHON, + true ) expect(mockedAppendFileSync).toHaveBeenCalledWith( @@ -247,7 +251,8 @@ describe('findOrCreateDvcYamlFile', () => { '/dir/my_project/', '/dir/my_project/src/training/train.py', 'train', - scriptCommand.PYTHON + scriptCommand.PYTHON, + true ) expect(mockedAppendFileSync).toHaveBeenCalledWith( @@ -259,7 +264,8 @@ describe('findOrCreateDvcYamlFile', () => { '/dir/my_project/', '/dir/my_other_project/train.py', 'train', - scriptCommand.PYTHON + scriptCommand.PYTHON, + true ) expect(mockedAppendFileSync).toHaveBeenCalledWith( @@ -268,12 +274,32 @@ describe('findOrCreateDvcYamlFile', () => { ) }) + it('should not convert the script path to a relative path if applyRelativePath is set to false', () => { + mockedOpenTextDocument.mockResolvedValue({} as TextDocument) + + void findOrCreateDvcYamlFile( + '/dir/my_project/', + join('dir', 'my_project', 'src', 'training', 'train.py'), + 'train', + scriptCommand.PYTHON, + false + ) + + expect(mockedAppendFileSync).toHaveBeenCalledWith( + expect.anything(), + expect.stringContaining( + join('dir', 'my_project', 'src', 'training', 'train.py') + ) + ) + }) + it('should use the jupyter nbconvert command if the training script is a Jupyter notebook', () => { void findOrCreateDvcYamlFile( '/', '/train.ipynb', 'train', - scriptCommand.JUPYTER + scriptCommand.JUPYTER, + true ) expect(mockedAppendFileSync).toHaveBeenCalledWith( @@ -291,7 +317,8 @@ describe('findOrCreateDvcYamlFile', () => { '/', '/train.py', 'train', - scriptCommand.PYTHON + scriptCommand.PYTHON, + true ) expect(mockedAppendFileSync).not.toHaveBeenCalledWith( @@ -306,7 +333,7 @@ describe('findOrCreateDvcYamlFile', () => { it('should use the custom command received', () => { const command = 'specialCommand' - void findOrCreateDvcYamlFile('/', '/train.other', 'train', command) + void findOrCreateDvcYamlFile('/', '/train.other', 'train', command, true) expect(mockedAppendFileSync).toHaveBeenCalledWith( expect.anything(), @@ -321,7 +348,8 @@ describe('findOrCreateDvcYamlFile', () => { '/', '/train.py', 'train', - scriptCommand.PYTHON + scriptCommand.PYTHON, + true ) expect(mockedOpenTextDocument).toHaveBeenCalledWith( diff --git a/extension/src/fileSystem/index.ts b/extension/src/fileSystem/index.ts index f22d527dc6..5035dc0977 100644 --- a/extension/src/fileSystem/index.ts +++ b/extension/src/fileSystem/index.ts @@ -1,4 +1,13 @@ -import { basename, extname, join, parse, relative, resolve, sep } from 'path' +import { + basename, + extname, + join, + parse, + relative, + resolve, + sep, + format +} from 'path' import { appendFileSync, ensureFileSync, @@ -153,21 +162,24 @@ export const findOrCreateDvcYamlFile = ( cwd: string, trainingScript: string, stageName: string, - command: string + command: string, + applyRelativePath: boolean ) => { const dvcYamlPath = `${cwd}/dvc.yaml` ensureFileSync(dvcYamlPath) - const relativeScript = relative(cwd, trainingScript) + const scriptPath = applyRelativePath + ? relative(cwd, trainingScript) + : format(parse(trainingScript)) const pipeline = ` # Read about DVC pipeline configuration (https://dvc.org/doc/user-guide/project-structure/dvcyaml-files#stages) # to customize your stages even more stages: ${stageName}: - cmd: ${command} ${relativeScript} + cmd: ${command} ${scriptPath} deps: - - ${relativeScript}` + - ${scriptPath}` void openFileInEditor(dvcYamlPath) return appendFileSync(dvcYamlPath, pipeline) diff --git a/languageServer/package.json b/languageServer/package.json index f1f0b6f3ae..6a284aa933 100644 --- a/languageServer/package.json +++ b/languageServer/package.json @@ -32,7 +32,7 @@ "copy-webpack-plugin": "11.0.0", "fork-ts-checker-webpack-plugin": "8.0.0", "ts-loader": "9.4.2", - "lint-staged": "13.2.0", + "lint-staged": "13.2.1", "jest": "29.5.0", "webpack": "5.78.0", "webpack-cli": "5.0.1", diff --git a/package.json b/package.json index bc44876013..1fb67675c1 100644 --- a/package.json +++ b/package.json @@ -37,7 +37,7 @@ "@typescript-eslint/eslint-plugin": "5.57.1", "@typescript-eslint/parser": "5.57.1", "@vscode/codicons": "0.0.32", - "eslint": "8.37.0", + "eslint": "8.38.0", "eslint-config-prettier": "8.8.0", "eslint-config-prettier-standard": "4.0.1", "eslint-config-standard": "17.0.0", @@ -58,14 +58,14 @@ "eslint-plugin-unicorn": "46.0.0", "husky": "8.0.3", "jest": "29.5.0", - "lint-staged": "13.2.0", + "lint-staged": "13.2.1", "npm-run-all": "4.1.5", "nyc": "15.1.0", "prettier": "2.8.7", "prettier-config-standard": "5.0.0", "ts-node": "10.9.1", "turbo": "1.8.8", - "typescript": "5.0.3" + "typescript": "5.0.4" }, "resolutions": { "decode-uri-component": "0.2.2", diff --git a/webview/package.json b/webview/package.json index b9c7d8a14d..d466bd1307 100644 --- a/webview/package.json +++ b/webview/package.json @@ -33,7 +33,7 @@ "react-intersection-observer": "9.4.3", "react-redux": "8.0.5", "react-vega": "7.6.0", - "react-virtualized": "9.22.3", + "react-virtualized": "9.22.4", "vega-lite": "5.6.1", "vega-util": "1.17.1" }, @@ -68,9 +68,9 @@ "jest": "29.5.0", "jest-canvas-mock": "2.5.0", "jest-environment-jsdom": "29.5.0", - "lint-staged": "13.2.0", + "lint-staged": "13.2.1", "raw-loader": "4.0.2", - "sass": "1.60.0", + "sass": "1.61.0", "sass-loader": "13.2.2", "storybook-addon-designs": "6.3.1", "storybook-addon-themes": "6.1.0", diff --git a/webview/src/experiments/components/table/Table.test.tsx b/webview/src/experiments/components/table/Table.test.tsx index a518fef1a8..3332198d16 100644 --- a/webview/src/experiments/components/table/Table.test.tsx +++ b/webview/src/experiments/components/table/Table.test.tsx @@ -400,5 +400,52 @@ describe('Table', () => { header?.classList.contains(styles.headerCellDropTarget) ).toBeFalsy() }) + + it('split group should be moving properly', async () => { + // test for https://github.com/iterative/vscode-dvc/issues/3679 + const { getDraggableHeaderFromText } = renderExperimentsTable({ + ...tableDataFixture + }) + + let headers = await getHeaders() + + // splits metrics and params group into 4 subgroups + dragAndDrop( + screen.getByText('val_accuracy'), + getDraggableHeaderFromText('learning_rate'), + DragEnterDirection.AUTO + ) + + headers = await getHeaders() + + expect(headers.indexOf('val_accuracy')).toBeGreaterThan( + headers.indexOf('learning_rate') + ) + + // note: moving the group here, not the leaf column + let metricGroups = screen.getAllByText('summary.json') + expect(metricGroups.length).toBe(2) + + dragAndDrop( + metricGroups[1], + getDraggableHeaderFromText('learning_rate'), + DragEnterDirection.AUTO + ) + + headers = await getHeaders() + + metricGroups = screen.getAllByText('summary.json') + const paramsGroups = screen.getAllByText('params.yaml') + + expect(metricGroups.length).toBe(2) + expect(paramsGroups.length).toBe(2) + + expect(headers.indexOf('val_accuracy')).toBeLessThan( + headers.indexOf('learning_rate') + ) + expect(headers.indexOf('epochs')).toBeGreaterThan( + headers.indexOf('val_loss') + ) + }) }) }) diff --git a/webview/src/experiments/util/columns.ts b/webview/src/experiments/util/columns.ts index e383d8e23b..447b36224c 100644 --- a/webview/src/experiments/util/columns.ts +++ b/webview/src/experiments/util/columns.ts @@ -37,7 +37,10 @@ export const reorderColumnIds = ( export const leafColumnIds = ( header: Header ): string[] => { - return header.column.getLeafColumns().map(col => col.id) + return header + .getLeafHeaders() + .filter(h => h.subHeaders.length === 0) + .map(h => h.column.id) } export const EXPERIMENT_COLUMN_ID = 'id' diff --git a/yarn.lock b/yarn.lock index 6de3f30e6e..9cb085a75b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1708,10 +1708,10 @@ minimatch "^3.1.2" strip-json-comments "^3.1.1" -"@eslint/js@8.37.0": - version "8.37.0" - resolved "https://registry.yarnpkg.com/@eslint/js/-/js-8.37.0.tgz#cf1b5fa24217fe007f6487a26d765274925efa7d" - integrity sha512-x5vzdtOOGgFVDCUs81QRB2+liax8rFg3+7hqM+QhBG0/G3F1ZsoYl97UrqgHgQ9KKT7G6c4V+aTUCgu/n22v1A== +"@eslint/js@8.38.0": + version "8.38.0" + resolved "https://registry.yarnpkg.com/@eslint/js/-/js-8.38.0.tgz#73a8a0d8aa8a8e6fe270431c5e72ae91b5337892" + integrity sha512-IoD2MfUnOV58ghIHCiil01PcohxjbYR/qCxsoC+xNgUwh1EY8jOOrYmu3d3a71+tJJ23uscEV4X2HJWMsPJu4g== "@fastify/accept-negotiator@^1.0.0": version "1.1.0" @@ -9499,15 +9499,15 @@ eslint-visitor-keys@^3.4.0: resolved "https://registry.yarnpkg.com/eslint-visitor-keys/-/eslint-visitor-keys-3.4.0.tgz#c7f0f956124ce677047ddbc192a68f999454dedc" integrity sha512-HPpKPUBQcAsZOsHAFwTtIKcYlCje62XB7SEAcxjtmW6TD1WVpkS6i6/hOVtTZIl4zGj/mBqpFVGvaDneik+VoQ== -eslint@8.37.0: - version "8.37.0" - resolved "https://registry.yarnpkg.com/eslint/-/eslint-8.37.0.tgz#1f660ef2ce49a0bfdec0b0d698e0b8b627287412" - integrity sha512-NU3Ps9nI05GUoVMxcZx1J8CNR6xOvUT4jAUMH5+z8lpp3aEdPVCImKw6PWG4PY+Vfkpr+jvMpxs/qoE7wq0sPw== +eslint@8.38.0: + version "8.38.0" + resolved "https://registry.yarnpkg.com/eslint/-/eslint-8.38.0.tgz#a62c6f36e548a5574dd35728ac3c6209bd1e2f1a" + integrity sha512-pIdsD2jwlUGf/U38Jv97t8lq6HpaU/G9NKbYmpWpZGw3LdTNhZLbJePqxOXGB5+JEKfOPU/XLxYxFh03nr1KTg== dependencies: "@eslint-community/eslint-utils" "^4.2.0" "@eslint-community/regexpp" "^4.4.0" "@eslint/eslintrc" "^2.0.2" - "@eslint/js" "8.37.0" + "@eslint/js" "8.38.0" "@humanwhocodes/config-array" "^0.11.8" "@humanwhocodes/module-importer" "^1.0.1" "@nodelib/fs.walk" "^1.2.8" @@ -13286,10 +13286,10 @@ linkify-it@^3.0.1: dependencies: uc.micro "^1.0.1" -lint-staged@13.2.0: - version "13.2.0" - resolved "https://registry.yarnpkg.com/lint-staged/-/lint-staged-13.2.0.tgz#b7abaf79c91cd36d824f17b23a4ce5209206126a" - integrity sha512-GbyK5iWinax5Dfw5obm2g2ccUiZXNGtAS4mCbJ0Lv4rq6iEtfBSjOYdcbOtAIFtM114t0vdpViDDetjVTSd8Vw== +lint-staged@13.2.1: + version "13.2.1" + resolved "https://registry.yarnpkg.com/lint-staged/-/lint-staged-13.2.1.tgz#9d30a14e3e42897ef417bc98556fb757f75cae87" + integrity sha512-8gfzinVXoPfga5Dz/ZOn8I2GOhf81Wvs+KwbEXQn/oWZAvCVS2PivrXfVbFJc93zD16uC0neS47RXHIjXKYZQw== dependencies: chalk "5.2.0" cli-truncate "^3.1.0" @@ -16117,10 +16117,10 @@ react-vega@7.6.0: prop-types "^15.8.1" vega-embed "^6.5.1" -react-virtualized@9.22.3: - version "9.22.3" - resolved "https://registry.yarnpkg.com/react-virtualized/-/react-virtualized-9.22.3.tgz#f430f16beb0a42db420dbd4d340403c0de334421" - integrity sha512-MKovKMxWTcwPSxE1kK1HcheQTWfuCxAuBoSTf2gwyMM21NdX/PXUhnoP8Uc5dRKd+nKm8v41R36OellhdCpkrw== +react-virtualized@9.22.4: + version "9.22.4" + resolved "https://registry.yarnpkg.com/react-virtualized/-/react-virtualized-9.22.4.tgz#ca1997d9677d520e742b2aec1463712c3e322e52" + integrity sha512-HYOQEK1OWWYKt8C3KnjbEOST224kv5D8USPSdo/huAamDttWzQLTtoi/IvWfqwFOr9cCnwUYzqOTStwPrdkqqQ== dependencies: "@babel/runtime" "^7.7.2" clsx "^1.0.4" @@ -16831,10 +16831,10 @@ sass-loader@13.2.2: klona "^2.0.6" neo-async "^2.6.2" -sass@1.60.0: - version "1.60.0" - resolved "https://registry.yarnpkg.com/sass/-/sass-1.60.0.tgz#657f0c23a302ac494b09a5ba8497b739fb5b5a81" - integrity sha512-updbwW6fNb5gGm8qMXzVO7V4sWf7LMXnMly/JEyfbfERbVH46Fn6q02BX7/eHTdKpE7d+oTkMMQpFWNUMfFbgQ== +sass@1.61.0: + version "1.61.0" + resolved "https://registry.yarnpkg.com/sass/-/sass-1.61.0.tgz#d1f6761bb833887b8fdab32a24e052c40531d02b" + integrity sha512-PDsN7BrVkNZK2+dj/dpKQAWZavbAQ87IXqVvw2+oEYI+GwlTWkvbQtL7F2cCNbMqJEYKPh1EcjSxsnqIb/kyaQ== dependencies: chokidar ">=3.0.0 <4.0.0" immutable "^4.0.0" @@ -18536,10 +18536,10 @@ typedarray@^0.0.6: resolved "https://registry.yarnpkg.com/typedarray/-/typedarray-0.0.6.tgz#867ac74e3864187b1d3d47d996a78ec5c8830777" integrity sha1-hnrHTjhkGHsdPUfZlqeOxciDB3c= -typescript@5.0.3: - version "5.0.3" - resolved "https://registry.yarnpkg.com/typescript/-/typescript-5.0.3.tgz#fe976f0c826a88d0a382007681cbb2da44afdedf" - integrity sha512-xv8mOEDnigb/tN9PSMTwSEqAnUvkoXMQlicOb0IUVDBSQCgBSaAAROUZYy2IcUy5qU6XajK5jjjO7TMWqBTKZA== +typescript@5.0.4: + version "5.0.4" + resolved "https://registry.yarnpkg.com/typescript/-/typescript-5.0.4.tgz#b217fd20119bd61a94d4011274e0ab369058da3b" + integrity sha512-cW9T5W9xY37cc+jfEnaUvX91foxtHkza3Nw3wkoF4sSlKn0MONdkdEndig/qPBWXNkmplh3NzayQzCiHM4/hqw== ua-parser-js@^1.0.1: version "1.0.33"