Accomodate params that are lists (#1818)

* add arrays to exp show data type

* return string version of the array to the experiments table to match CLI

* ensure we pass the right data when using modify and commands

* unit test and improve quick pick

* add string and boolean data types

Author: mattseddon

extension/src/cli/reader.ts

@@ -49,7 +49,14 @@ export type StatusesOrAlwaysChanged = StageOrFileStatuses | 'always changed'
 
 export type StatusOutput = Record<string, StatusesOrAlwaysChanged[]>
 
-export type Value = string | number | boolean | null
+export type Value =
+  | string
+  | number
+  | boolean
+  | null
+  | number[]
+  | string[]
+  | boolean[]
 
 export interface ValueTreeOrError {
   data?: ValueTree

extension/src/experiments/columns/collect.test.ts

@@ -416,6 +416,7 @@ describe('collectColumns', () => {
         joinColumnPath(ColumnType.METRICS, 'summary.json', 'val_loss'),
         joinColumnPath(ColumnType.METRICS, 'summary.json', 'val_accuracy'),
         joinColumnPath(ColumnType.PARAMS, 'params.yaml'),
+        joinColumnPath(ColumnType.PARAMS, 'params.yaml', 'code_names'),
         joinColumnPath(ColumnType.PARAMS, 'params.yaml', 'epochs'),
         joinColumnPath(ColumnType.PARAMS, 'params.yaml', 'learning_rate'),
         joinColumnPath(ColumnType.PARAMS, 'params.yaml', 'dvc_logs_dir'),

extension/src/experiments/columns/collect.ts

@@ -17,6 +17,9 @@ const getValueType = (value: Value) => {
   if (value === null) {
     return 'null'
   }
+  if (Array.isArray(value)) {
+    return 'array'
+  }
   return typeof value
 }
 

extension/src/experiments/columns/tree.test.ts

@@ -217,6 +217,14 @@ describe('ExperimentsColumnsTree', () => {
         path: paramsPath
       })
       expect(grandChildren).toStrictEqual([
+        {
+          collapsibleState: 0,
+          description: undefined,
+          dvcRoot: mockedDvcRoot,
+          iconPath: mockedSelectedCheckbox,
+          label: 'code_names',
+          path: appendColumnToPath(paramsPath, 'code_names')
+        },
         {
           collapsibleState: 0,
           description: undefined,

extension/src/experiments/columns/walk.ts

@@ -27,7 +27,7 @@ const walkValueTree = (
   ancestors: string[] = []
 ) => {
   for (const [key, value] of Object.entries(tree)) {
-    if (value && typeof value === 'object') {
+    if (value && !Array.isArray(value) && typeof value === 'object') {
       walkValueTree(value, meta, onValue, [...ancestors, key])
     } else {
       onValue(key, value, meta, ancestors)

extension/src/experiments/model/queue/collect.test.ts

@@ -7,6 +7,7 @@ describe('collectFlatExperimentParams', () => {
   it('should flatten the params into an array', () => {
     const params = collectFlatExperimentParams(rowsFixture[0].params)
     expect(params).toStrictEqual([
+      { path: appendColumnToPath('params.yaml', 'code_names'), value: [0, 1] },
       { path: appendColumnToPath('params.yaml', 'epochs'), value: 2 },
       {
         path: appendColumnToPath('params.yaml', 'learning_rate'),

extension/src/experiments/model/queue/collect.ts

@@ -4,18 +4,18 @@ import { Columns } from '../../webview/contract'
 
 export type Param = {
   path: string
-  value: number | string | boolean
+  value: Value
 }
 
 const collectFromParamsFile = (
-  acc: { path: string; value: string | number | boolean }[],
+  acc: { path: string; value: Value }[],
   key: string | undefined,
   value: Value | ValueTree,
   ancestors: string[] = []
 ) => {
   const pathArray = [...ancestors, key].filter(Boolean) as string[]
 
-  if (typeof value === 'object') {
+  if (!Array.isArray(value) && typeof value === 'object') {
     for (const [childKey, childValue] of Object.entries(value as ValueTree)) {
       collectFromParamsFile(acc, childKey, childValue, pathArray)
     }

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

@@ -48,27 +48,38 @@ describe('pickAndModifyParams', () => {
     const unchanged = { path: 'params.yaml:learning_rate', value: 2e-12 }
     const initialUserResponse = [
       { path: 'params.yaml:dropout', value: 0.15 },
-      { path: 'params.yaml:process.threshold', value: 0.86 }
+      { path: 'params.yaml:process.threshold', value: 0.86 },
+      { path: 'params.yaml:code_names', value: [0, 1, 2] }
     ]
     mockedQuickPickManyValues.mockResolvedValueOnce(initialUserResponse)
     const firstInput = '0.16'
     const secondInput = '0.87'
+    const thirdInput = '[0,1,3]'
     mockedGetInput.mockResolvedValueOnce(firstInput)
     mockedGetInput.mockResolvedValueOnce(secondInput)
+    mockedGetInput.mockResolvedValueOnce(thirdInput)
 
     const paramsToQueue = await pickAndModifyParams([
       unchanged,
       ...initialUserResponse
     ])
 
+    expect(mockedGetInput).toBeCalledTimes(3)
+    expect(mockedGetInput).toBeCalledWith(
+      'Enter a Value for params.yaml:code_names',
+      '[0,1,2]'
+    )
+
     expect(paramsToQueue).toStrictEqual([
       '-S',
       `params.yaml:dropout=${firstInput}`,
       '-S',
       `params.yaml:process.threshold=${secondInput}`,
       '-S',
+      `params.yaml:code_names=${thirdInput}`,
+      '-S',
       [unchanged.path, unchanged.value].join('=')
     ])
-    expect(mockedGetInput).toBeCalledTimes(2)
+    expect(mockedGetInput).toBeCalledTimes(3)
   })
 })

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

@@ -4,11 +4,15 @@ import { getInput } from '../../../vscode/inputBox'
 import { Flag } from '../../../cli/constants'
 import { definedAndNonEmpty } from '../../../util/array'
 import { getEnterValueTitle, Title } from '../../../vscode/title'
+import { Value } from '../../../cli/reader'
+
+const standardizeValue = (value: Value): string =>
+  typeof value === 'object' ? JSON.stringify(value) : `${value}`
 
 const pickParamsToModify = (params: Param[]): Thenable<Param[] | undefined> =>
   quickPickManyValues<Param>(
     params.map(param => ({
-      description: `${param.value}`,
+      description: standardizeValue(param.value),
       label: param.path,
       picked: false,
       value: param
@@ -21,18 +25,21 @@ const pickNewParamValues = async (
 ): Promise<string[] | undefined> => {
   const args: string[] = []
   for (const { path, value } of paramsToModify) {
-    const input = await getInput(getEnterValueTitle(path), `${value}`)
+    const input = await getInput(
+      getEnterValueTitle(path),
+      standardizeValue(value)
+    )
     if (input === undefined) {
       return
     }
-    args.push(Flag.SET_PARAM, [path, input.trim()].join('='))
+    args.push(Flag.SET_PARAM, [path, standardizeValue(input.trim())].join('='))
   }
   return args
 }
 
 const addUnchanged = (args: string[], unchanged: Param[]) => {
   for (const { path, value } of unchanged) {
-    args.push(Flag.SET_PARAM, [path, value].join('='))
+    args.push(Flag.SET_PARAM, [path, standardizeValue(value)].join('='))
   }
 
   return args

extension/src/test/fixtures/expShow/columns.ts

@@ -67,6 +67,16 @@ const data: Column[] = [
     parentPath: ColumnType.PARAMS,
     path: joinColumnPath(ColumnType.PARAMS, 'params.yaml')
   },
+  {
+    type: ColumnType.PARAMS,
+    hasChildren: false,
+    maxStringLength: 3,
+    name: 'code_names',
+    parentPath: joinColumnPath(ColumnType.PARAMS, 'params.yaml'),
+    path: joinColumnPath(ColumnType.PARAMS, 'params.yaml', 'code_names'),
+    pathArray: [ColumnType.PARAMS, 'params.yaml', 'code_names'],
+    types: ['array']
+  },
   {
     type: ColumnType.PARAMS,
     hasChildren: false,