Skip to content

Commit d5c8800

Browse files
authored
Only use selected params for modify and commands (#4095)
1 parent 01ead14 commit d5c8800

File tree

2 files changed

+9
-76
lines changed

2 files changed

+9
-76
lines changed

extension/src/experiments/model/modify/quickPick.test.ts

Lines changed: 1 addition & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -109,57 +109,7 @@ describe('pickAndModifyParams', () => {
109109
'-S',
110110
`params.yaml:code_names=${thirdInput}`,
111111
'-S',
112-
`params.yaml:transforms='${fourthInput}'`,
113-
'-S',
114-
[unchanged.path, unchanged.value].join('=')
115-
])
116-
})
117-
118-
it('should convert any unselected params into the required format', async () => {
119-
const numericValue = {
120-
isString: false,
121-
path: 'params.yaml:learning_rate',
122-
value: 2e-12
123-
}
124-
const stringArrayValue = {
125-
isString: true,
126-
path: 'params.yaml:transforms',
127-
value: '[Pipeline: PILBase.create, Pipeline: partial -> PILBase.create]'
128-
}
129-
130-
const actualArrayValue = {
131-
isString: false,
132-
path: 'params.yaml:code_names',
133-
value: [0, 1, 2]
134-
}
135-
136-
const unchanged = [numericValue, stringArrayValue, actualArrayValue]
137-
const initialUserResponse = {
138-
isString: false,
139-
path: 'params.yaml:dropout',
140-
value: 0.15
141-
}
142-
143-
mockedQuickPickManyValues.mockResolvedValueOnce([initialUserResponse])
144-
const firstInput = '0.16'
145-
mockedGetInput.mockResolvedValueOnce(firstInput)
146-
147-
const paramsToQueue = await pickAndModifyParams([
148-
...unchanged,
149-
initialUserResponse
150-
])
151-
152-
expect(mockedGetInput).toHaveBeenCalledTimes(1)
153-
154-
expect(paramsToQueue).toStrictEqual([
155-
'-S',
156-
`params.yaml:dropout=${firstInput}`,
157-
'-S',
158-
[numericValue.path, numericValue.value].join('='),
159-
'-S',
160-
[stringArrayValue.path, `'${stringArrayValue.value}'`].join('='),
161-
'-S',
162-
[actualArrayValue.path, JSON.stringify(actualArrayValue.value)].join('=')
112+
`params.yaml:transforms='${fourthInput}'`
163113
])
164114
})
165115
})

extension/src/experiments/model/modify/quickPick.ts

Lines changed: 8 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,11 @@ import { definedAndNonEmpty } from '../../../util/array'
66
import { getEnterValueTitle, Title } from '../../../vscode/title'
77
import { Value } from '../../../cli/dvc/contract'
88

9-
const standardizeValue = (value: Value, isString: boolean): string => {
10-
if (isString && typeof value === 'string') {
9+
const standardizeValue = (
10+
value: Value,
11+
wrapStringParamForCli = false
12+
): string => {
13+
if (wrapStringParamForCli && typeof value === 'string') {
1114
return `'${value}'`
1215
}
1316
return typeof value === 'object' ? JSON.stringify(value) : String(value)
@@ -18,7 +21,7 @@ const pickParamsToModify = (
1821
): Thenable<ParamWithIsString[] | undefined> =>
1922
quickPickManyValues<ParamWithIsString>(
2023
params.map(param => ({
21-
description: standardizeValue(param.value, false),
24+
description: standardizeValue(param.value),
2225
label: param.path,
2326
picked: false,
2427
value: param
@@ -33,7 +36,7 @@ const pickNewParamValues = async (
3336
for (const { path, value, isString } of paramsToModify) {
3437
const input = await getInput(
3538
getEnterValueTitle(path),
36-
standardizeValue(value, false)
39+
standardizeValue(value)
3740
)
3841
if (input === undefined) {
3942
return
@@ -46,17 +49,6 @@ const pickNewParamValues = async (
4649
return args
4750
}
4851

49-
const addUnchanged = (args: string[], unchanged: ParamWithIsString[]) => {
50-
for (const { path, value, isString } of unchanged) {
51-
args.push(
52-
Flag.SET_PARAM,
53-
[path, standardizeValue(value, isString)].join('=')
54-
)
55-
}
56-
57-
return args
58-
}
59-
6052
export const pickAndModifyParams = async (
6153
params: ParamWithIsString[]
6254
): Promise<string[] | undefined> => {
@@ -66,14 +58,5 @@ export const pickAndModifyParams = async (
6658
return
6759
}
6860

69-
const args = await pickNewParamValues(paramsToModify)
70-
71-
if (!args) {
72-
return
73-
}
74-
75-
const paramPathsToModify = new Set(paramsToModify.map(param => param.path))
76-
const unchanged = params.filter(param => !paramPathsToModify.has(param.path))
77-
78-
return addUnchanged(args, unchanged)
61+
return pickNewParamValues(paramsToModify)
7962
}

0 commit comments

Comments
 (0)