From c496306f88965b7a1d4290cc154950b6ff4d44a2 Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Fri, 27 Jan 2023 13:53:07 +1100 Subject: [PATCH] combine remove experiments and remove queued experiment --- extension/package.json | 13 +----- extension/package.nls.json | 3 +- extension/src/commands/external.ts | 3 +- .../src/experiments/commands/register.ts | 5 --- extension/src/experiments/index.ts | 11 +---- extension/src/experiments/model/index.ts | 45 +++++++------------ extension/src/experiments/workspace.ts | 6 --- extension/src/telemetry/constants.ts | 1 - .../test/suite/experiments/workspace.test.ts | 41 ++++++----------- 9 files changed, 36 insertions(+), 92 deletions(-) diff --git a/extension/package.json b/extension/package.json index 115e81ac09..f943d242c8 100644 --- a/extension/package.json +++ b/extension/package.json @@ -288,7 +288,7 @@ }, { "title": "%command.removeExperiments%", - "command": "dvc.removeExperiment", + "command": "dvc.removeExperiments", "category": "DVC" }, { @@ -307,11 +307,6 @@ "category": "DVC", "icon": "$(close-all)" }, - { - "title": "%command.removeQueuedExperiment%", - "command": "dvc.removeQueuedExperiment", - "category": "DVC" - }, { "title": "%command.removeTarget%", "command": "dvc.removeTarget", @@ -740,7 +735,7 @@ "when": "dvc.commands.available && dvc.project.available && !dvc.experiment.running" }, { - "command": "dvc.removeExperiment", + "command": "dvc.removeExperiments", "when": "dvc.commands.available && dvc.project.available && !dvc.experiment.running" }, { @@ -755,10 +750,6 @@ "command": "dvc.removeExperimentsTableSorts", "when": "dvc.commands.available && dvc.project.available" }, - { - "command": "dvc.removeQueuedExperiment", - "when": "dvc.commands.available && dvc.project.available && !dvc.experiment.running" - }, { "command": "dvc.removeTarget", "when": "false" diff --git a/extension/package.nls.json b/extension/package.nls.json index 8921c9171d..4a5e22a10e 100644 --- a/extension/package.nls.json +++ b/extension/package.nls.json @@ -35,11 +35,10 @@ "command.modifyExperimentParamsAndResume": "Modify Experiment Param(s) and Resume", "command.modifyExperimentParamsResetAndRun": "Modify Experiment Param(s) and Run", "command.queueExperiment": "Queue Experiment", - "command.removeExperiments": "Remove Experiments", + "command.removeExperiments": "Remove Experiment(s)", "command.removeExperimentQueue": "Remove All Queued Experiments", "command.removeExperimentsTableFilters": "Remove Filter(s) From Experiments Table", "command.removeExperimentsTableSorts": "Remove Sort(s) From Experiments Table", - "command.removeQueuedExperiment": "Remove Queued Experiment", "command.removeTarget": "Remove", "command.renameTarget": "Rename", "command.runExperiment": "Run Experiment", diff --git a/extension/src/commands/external.ts b/extension/src/commands/external.ts index 5e5c96fbee..355f052936 100644 --- a/extension/src/commands/external.ts +++ b/extension/src/commands/external.ts @@ -2,9 +2,8 @@ export enum RegisteredCliCommands { EXPERIMENT_APPLY = 'dvc.applyExperiment', EXPERIMENT_BRANCH = 'dvc.branchExperiment', EXPERIMENT_GARBAGE_COLLECT = 'dvc.experimentGarbageCollect', - EXPERIMENT_REMOVE = 'dvc.removeExperiment', + EXPERIMENT_REMOVE = 'dvc.removeExperiments', EXPERIMENT_REMOVE_QUEUE = 'dvc.removeExperimentQueue', - EXPERIMENT_REMOVE_QUEUED = 'dvc.removeQueuedExperiment', EXPERIMENT_RESET_AND_RUN = 'dvc.resetAndRunCheckpointExperiment', EXPERIMENT_RESUME = 'dvc.resumeCheckpointExperiment', EXPERIMENT_RUN = 'dvc.runExperiment', diff --git a/extension/src/experiments/commands/register.ts b/extension/src/experiments/commands/register.ts index 4acb2c5b7a..2b0d2b5670 100644 --- a/extension/src/experiments/commands/register.ts +++ b/extension/src/experiments/commands/register.ts @@ -143,11 +143,6 @@ const registerExperimentNameCommands = ( ...ids ) ) - - internalCommands.registerExternalCliCommand( - RegisteredCliCommands.EXPERIMENT_REMOVE_QUEUED, - () => experiments.getQueuedExpThenRun(AvailableCommands.EXPERIMENT_REMOVE) - ) } const registerExperimentInputCommands = ( diff --git a/extension/src/experiments/index.ts b/extension/src/experiments/index.ts index 604cccfad5..8a44941354 100644 --- a/extension/src/experiments/index.ts +++ b/extension/src/experiments/index.ts @@ -387,14 +387,7 @@ export class Experiments extends BaseRepository { public pickCurrentExperiment() { return pickExperiment( - this.experiments.getCurrentExperiments(), - this.getFirstThreeColumnOrder() - ) - } - - public pickQueuedExperiment() { - return pickExperiment( - this.experiments.getQueuedExperiments(), + this.experiments.getExperimentsWithoutQueued(), this.getFirstThreeColumnOrder() ) } @@ -409,7 +402,7 @@ export class Experiments extends BaseRepository { public pickExperimentsToRemove() { return pickExperiments( - this.experiments.getCurrentExperiments(), + this.experiments.getExperimentsWithQueued(), this.getFirstThreeColumnOrder(), Title.SELECT_EXPERIMENTS_REMOVE ) diff --git a/extension/src/experiments/model/index.ts b/extension/src/experiments/model/index.ts index 29f562a3c4..d90d29f764 100644 --- a/extension/src/experiments/model/index.ts +++ b/extension/src/experiments/model/index.ts @@ -148,7 +148,7 @@ export class ExperimentsModel extends ModelWithPersistence { public toggleStatus(id: string) { if ( isQueued( - this.getFlattenedExperiments().find( + this.getExperimentsWithQueued().find( ({ id: queuedId }) => queuedId === id )?.status ) @@ -185,7 +185,7 @@ export class ExperimentsModel extends ModelWithPersistence { } public setRevisionCollected(revisions: string[]) { - for (const { id } of this.getFlattenedExperiments().filter(({ label }) => + for (const { id } of this.getExperimentsWithQueued().filter(({ label }) => revisions.includes(label) )) { if (this.finishedRunning[id]) { @@ -281,7 +281,7 @@ export class ExperimentsModel extends ModelWithPersistence { } public getSelectedExperiments() { - return this.getSelectedFromList(() => this.getFlattenedExperiments()) + return this.getSelectedFromList(() => this.getExperimentsWithQueued()) } public setSelected(selectedExperiments: Experiment[]) { @@ -348,7 +348,7 @@ export class ExperimentsModel extends ModelWithPersistence { } public getAllExperiments() { - return [...this.getExperiments(), ...this.getFlattenedExperiments()] + return [...this.getExperiments(), ...this.getExperimentsWithQueued()] } public getErrors() { @@ -385,16 +385,20 @@ export class ExperimentsModel extends ModelWithPersistence { return collectFlatExperimentParams(params) } - public getCurrentExperiments() { - return this.splitExperimentsByQueued() + public getExperimentsWithoutQueued() { + return this.getExperimentsWithQueued().filter(({ status }) => { + return !isQueued(status) + }) } - public getQueuedExperiments() { - return this.splitExperimentsByQueued(true) + public getExperimentsWithQueued() { + return flattenMapValues(this.experimentsByBranch).map(experiment => + this.addDetails(experiment) + ) } public getRunningQueueTasks() { - return this.getFlattenedExperiments().filter( + return this.getExperimentsWithQueued().filter( ({ status, executor }) => isRunning(status) && executor === 'dvc-task' ) } @@ -438,7 +442,7 @@ export class ExperimentsModel extends ModelWithPersistence { public getExperimentCount() { return sum([ this.getFlattenedCheckpoints().length, - this.getFlattenedExperiments().length, + this.getExperimentsWithQueued().length, this.branches.length, 1 ]) @@ -453,7 +457,7 @@ export class ExperimentsModel extends ModelWithPersistence { return [ this.workspace, ...this.branches, - ...this.getFlattenedExperiments(), + ...this.getExperimentsWithQueued(), ...this.getFlattenedCheckpoints() ] } @@ -505,7 +509,7 @@ export class ExperimentsModel extends ModelWithPersistence { private getFilteredExperiments() { const acc: ExperimentWithType[] = [] - for (const experiment of this.getCurrentExperiments()) { + for (const experiment of this.getExperimentsWithoutQueued()) { const checkpoints = this.getCheckpointsWithType(experiment.id) || [] collectFiltered(acc, this.getFilters(), experiment, checkpoints) } @@ -541,21 +545,6 @@ export class ExperimentsModel extends ModelWithPersistence { return sortExperiments(this.getSorts(), experiments) } - private getFlattenedExperiments() { - return flattenMapValues(this.experimentsByBranch).map(experiment => - this.addDetails(experiment) - ) - } - - private splitExperimentsByQueued(getQueued = false) { - return this.getFlattenedExperiments().filter(({ status }) => { - if (getQueued) { - return isQueued(status) - } - return !isQueued(status) - }) - } - private getFlattenedCheckpoints() { return flattenMapValues(this.checkpointsByTip).map(checkpoint => this.addDetails(checkpoint) @@ -593,7 +582,7 @@ export class ExperimentsModel extends ModelWithPersistence { this.finishedRunning = collectFinishedRunningExperiments( { ...this.finishedRunning }, - this.getFlattenedExperiments(), + this.getExperimentsWithQueued(), this.running, stillRunning, this.coloredStatus diff --git a/extension/src/experiments/workspace.ts b/extension/src/experiments/workspace.ts index 29c3d7cee0..7d4324cb5a 100644 --- a/extension/src/experiments/workspace.ts +++ b/extension/src/experiments/workspace.ts @@ -244,12 +244,6 @@ export class WorkspaceExperiments extends BaseWorkspaceWebviews< ) } - public getQueuedExpThenRun(commandId: CommandId) { - return this.pickExpThenRun(commandId, cwd => - this.getRepository(cwd).pickQueuedExperiment() - ) - } - public async getCwdAndQuickPickThenRun( commandId: CommandId, quickPick: () => Thenable diff --git a/extension/src/telemetry/constants.ts b/extension/src/telemetry/constants.ts index ed6b947679..f018ec2a4a 100644 --- a/extension/src/telemetry/constants.ts +++ b/extension/src/telemetry/constants.ts @@ -134,7 +134,6 @@ export interface IEventNamePropertyMapping { [EventName.EXPERIMENT_METRICS_AND_PARAMS_TOGGLE]: undefined [EventName.EXPERIMENT_REMOVE]: undefined [EventName.EXPERIMENT_REMOVE_QUEUE]: undefined - [EventName.EXPERIMENT_REMOVE_QUEUED]: undefined [EventName.EXPERIMENT_RESUME]: undefined [EventName.EXPERIMENT_RUN]: undefined [EventName.EXPERIMENT_RESET_AND_RUN]: undefined diff --git a/extension/src/test/suite/experiments/workspace.test.ts b/extension/src/test/suite/experiments/workspace.test.ts index 67f84eb492..743279ffb3 100644 --- a/extension/src/test/suite/experiments/workspace.test.ts +++ b/extension/src/test/suite/experiments/workspace.test.ts @@ -773,8 +773,8 @@ suite('Workspace Experiments Test Suite', () => { }) }) - describe('dvc.removeExperiment', () => { - it('should ask the user to pick experiment(s) and then remove selected experiments from the workspace', async () => { + describe('dvc.removeExperiments', () => { + it('should ask the user to pick experiment(s) and then remove selected ones from the workspace', async () => { const mockExperiment = 'exp-e7a67' const secondMockExperiment = 'exp-83425' type QuickPickReturnValue = QuickPickItemWithValue<{ @@ -867,6 +867,17 @@ suite('Workspace Experiments Test Suite', () => { label: 'f0f9186', value: { id: 'exp-f13bca', name: 'exp-f13bca' } }, + { + description: undefined, + detail: `Created:${formatDate( + '2020-12-29T15:25:27' + )}, loss:-, accuracy:-`, + label: '90aea7f', + value: { + id: '90aea7f2482117a55dfcadcdb901aaa6610fbbc9', + name: '90aea7f' + } + }, { description: undefined, detail: `Created:${formatDate( @@ -918,30 +929,4 @@ suite('Workspace Experiments Test Suite', () => { expect(mockExperimentRemove).to.be.calledWith(dvcDemoPath, '--queue') }) }) - - describe('dvc.removeQueuedExperiment', () => { - it('should ask the user to pick a queued experiment and then remove that experiment from the workspace', async () => { - const mockExperiment = 'queued-exp-to-remove' - - const { experiments } = buildExperiments(disposable) - - await experiments.isReady() - - stubWorkspaceExperimentsGetters(dvcDemoPath, experiments) - - stub(window, 'showQuickPick').resolves({ - value: { id: mockExperiment, name: mockExperiment } - } as QuickPickItemWithValue<{ id: string; name: string }>) - const mockExperimentRemove = stub( - DvcExecutor.prototype, - 'experimentRemove' - ) - - await commands.executeCommand( - RegisteredCliCommands.EXPERIMENT_REMOVE_QUEUED - ) - - expect(mockExperimentRemove).to.be.calledWith(dvcDemoPath, mockExperiment) - }) - }) })