Fix exp tree tooltips and quick pick selection ignoring columns with falsy values (#2745)

Author: julieg18

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

@@ -83,7 +83,7 @@ describe('pickExperiments', () => {
       label: '123fsf4',
       params: {
         'params.yaml': {
-          prepare: { split: 0.1 }
+          prepare: { split: 0 }
         }
       },
       selected: false
@@ -135,7 +135,7 @@ describe('pickExperiments', () => {
       [
         {
           description: '[exp-123]',
-          detail: 'Created:Aug 19, 2022, split:0.1, data/data.xml:22a1a29',
+          detail: 'Created:Aug 19, 2022, split:0, data/data.xml:22a1a29',
           label: '123fsf4',
           value: mockedExperiments[0]
         },

extension/src/experiments/model/quickPicks.ts

@@ -11,7 +11,7 @@ import {
 import { Toast } from '../../vscode/toast'
 import { Experiment } from '../webview/contract'
 import { Title } from '../../vscode/title'
-import { truncate, truncateFromLeft } from '../../util/string'
+import { truncateFromLeft } from '../../util/string'
 
 type QuickPickItemAccumulator = {
   items: QuickPickItemWithValue<Experiment | undefined>[]
@@ -32,10 +32,8 @@ const getItem = (experiment: Experiment, firstThreeColumnOrder: string[]) => ({
         splitUpPath[splitUpPath.length - 1],
         15
       )
-      const truncatedVal =
-        typeof value === 'number' ? truncate(String(value), 7) : value
 
-      return value ? `${truncatedKey}:${truncatedVal}` : ''
+      return value !== null ? `${truncatedKey}:${value}` : ''
     })
     .filter(Boolean)
     .join(', '),

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

@@ -407,7 +407,9 @@ describe('ExperimentsTree', () => {
           label: 'a123',
           params: {
             'params.yaml': {
-              featurize: { max_features: 200 }
+              featurize: {
+                random_value: undefined
+              }
             }
           },
           selected: false,
@@ -424,7 +426,9 @@ describe('ExperimentsTree', () => {
           label: 'b456',
           params: {
             'params.yaml': {
-              featurize: { max_features: 210 }
+              featurize: {
+                random_value: ['rbf', 'linear']
+              }
             }
           },
           selected: true,
@@ -441,7 +445,9 @@ describe('ExperimentsTree', () => {
           label: 'c789',
           params: {
             'params.yaml': {
-              featurize: { max_features: 190 }
+              featurize: {
+                random_value: false
+              }
             }
           },
           selected: false,
@@ -458,8 +464,8 @@ describe('ExperimentsTree', () => {
       mockedGetDvcRoots.mockReturnValueOnce(['repo'])
       mockedGetFirstThreeColumnOrder.mockReturnValue([
         'Created',
-        'params:params.yaml:featurize.max_features',
-        'deps:data/data.xml'
+        'deps:data/data.xml',
+        'params:params.yaml:featurize.random_value'
       ])
 
       const children = await experimentsTree.getChildren()
@@ -478,7 +484,7 @@ describe('ExperimentsTree', () => {
           id: 'exp-123',
           label: 'a123',
           tooltip:
-            '|||\n|:--|--|\n| Created | Aug 19, 2022 |\n| ...ms.yaml:featurize.max_features | 200 |\n| data/data.xml | 22a1a29 |\n',
+            '|||\n|:--|--|\n| Created | Aug 19, 2022 |\n| data/data.xml | 22a1a29 |\n| ...ms.yaml:featurize.random_value | undefined |\n',
           type: ExperimentType.EXPERIMENT
         },
         {
@@ -494,7 +500,7 @@ describe('ExperimentsTree', () => {
           id: 'exp-456',
           label: 'b456',
           tooltip:
-            '|||\n|:--|--|\n| Created | Sep 15, 2022 |\n| ...ms.yaml:featurize.max_features | 210 |\n| data/data.xml | 22a1a29 |\n',
+            '|||\n|:--|--|\n| Created | Sep 15, 2022 |\n| data/data.xml | 22a1a29 |\n| ...ms.yaml:featurize.random_value | [rbf,linear] |\n',
           type: ExperimentType.EXPERIMENT
         },
         {
@@ -510,7 +516,7 @@ describe('ExperimentsTree', () => {
           id: 'exp-789',
           label: 'c789',
           tooltip:
-            '|||\n|:--|--|\n| Created | Jul 3, 2022 |\n| ...ms.yaml:featurize.max_features | 190 |\n| data/data.xml | 22a1a29 |\n',
+            '|||\n|:--|--|\n| Created | Jul 3, 2022 |\n| data/data.xml | 22a1a29 |\n| ...ms.yaml:featurize.random_value | false |\n',
           type: ExperimentType.EXPERIMENT
         }
       ])

extension/src/experiments/model/tree.ts

@@ -33,7 +33,7 @@ import { sum } from '../../util/math'
 import { Disposable } from '../../class/dispose'
 import { Experiment, ExperimentStatus, isRunning } from '../webview/contract'
 import { getMarkdownString } from '../../vscode/markdownString'
-import { truncate, truncateFromLeft } from '../../util/string'
+import { truncateFromLeft } from '../../util/string'
 
 export class ExperimentsTree
   extends Disposable
@@ -355,9 +355,7 @@ export class ExperimentsTree
           path.slice(type.length + 1) || path,
           30
         )
-        const truncatedVal =
-          typeof value === 'number' ? truncate(String(value), 7) : value
-        return value ? `| ${truncatedKey} | ${truncatedVal} |\n` : ''
+        return value !== null ? `| ${truncatedKey} | ${value} |\n` : ''
       })
       .join('')
     return getMarkdownString(`|||\n|:--|--|\n${data}`)

extension/src/experiments/model/util.ts

@@ -1,20 +1,55 @@
 import get from 'lodash.get'
 import { formatDate } from '../../util/date'
+import { truncate } from '../../util/string'
 import { splitColumnPath } from '../columns/paths'
 import { Experiment } from '../webview/contract'
 
+type Value = undefined | null | [] | string | number
+
+const getValueType = (value: Value): string => {
+  let type: string = typeof value
+  if (Number.isNaN(value)) {
+    type = 'NaN'
+  } else if (typeof value === 'string' && Date.parse(value)) {
+    type = 'date'
+  } else if (Array.isArray(value)) {
+    type = 'array'
+  } else if (value === '') {
+    type = 'empty'
+  }
+  return type
+}
+
+const getStringifiedValue = (value: Value): string => {
+  const type = getValueType(value)
+  switch (type) {
+    case 'date':
+      return typeof value === 'string' ? formatDate(value) : ''
+    case 'array':
+      return `[${value?.toString()}]`
+    case 'empty':
+    case 'NaN':
+      return type
+    case 'number':
+      return truncate(String(value), 7)
+    default:
+      return String(value)
+  }
+}
+
 export const getDataFromColumnPath = (
   experiment: Experiment,
   columnPath: string
 ): { splitUpPath: string[]; value: string | null } => {
   const splitUpPath = splitColumnPath(columnPath)
   const collectedVal = get(experiment, splitUpPath)
-  const value =
-    columnPath === 'Created' && collectedVal
-      ? formatDate(collectedVal)
-      : collectedVal?.value || collectedVal
+  const value = collectedVal?.value || collectedVal
+
   return {
     splitUpPath,
-    value: value || null
+    value:
+      columnPath === 'Created' && typeof value === 'undefined'
+        ? null
+        : getStringifiedValue(value)
   }
 }