Skip to content

Commit a1486c7

Browse files
julieg18julieg18
authored
Refactor getDataFromColumnPath logic (#2971)
* simplify code and reduce duplication
1 parent 43496aa commit a1486c7

File tree

10 files changed

+88
-134
lines changed

10 files changed

+88
-134
lines changed

‎extension/src/experiments/model/quickPicks.ts‎

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { QuickPickItemKind } from 'vscode'
22
import omit from 'lodash.omit'
33
import { ExperimentWithCheckpoints } from '.'
44
import { MAX_SELECTED_EXPERIMENTS } from './status'
5-
import { getDataFromColumnPath } from './util'
5+
import { getDataFromColumnPaths } from './util'
66
import { definedAndNonEmpty } from '../../util/array'
77
import {
88
QuickPickItemWithValue,
@@ -25,17 +25,11 @@ const getSeparator = (experiment: Experiment) => ({
2525
})
2626

2727
const getItem = (experiment: Experiment, firstThreeColumnOrder: string[]) => ({
28-
detail: firstThreeColumnOrder
29-
.map(path => {
30-
const { splitUpPath, value } = getDataFromColumnPath(experiment, path)
31-
const truncatedKey = truncateFromLeft(
32-
splitUpPath[splitUpPath.length - 1],
33-
15
34-
)
35-
36-
return value === null ? '' : `${truncatedKey}:${value}`
37-
})
38-
.filter(Boolean)
28+
detail: getDataFromColumnPaths(experiment, firstThreeColumnOrder)
29+
.map(
30+
({ splitUpPath, truncatedValue: value }) =>
31+
`${truncateFromLeft(splitUpPath[splitUpPath.length - 1], 15)}:${value}`
32+
)
3933
.join(', '),
4034
label: experiment.label,
4135
value: omit(experiment, 'checkpoints')

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

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import { ExperimentType } from '.'
1212
import { ExperimentAugmented } from './filterBy/collect'
1313
import { collectDeletable, ExperimentItem } from './collect'
1414
import { MAX_SELECTED_EXPERIMENTS } from './status'
15-
import { getDataFromColumnPath } from './util'
15+
import { getDataFromColumnPaths } from './util'
1616
import { WorkspaceExperiments } from '../workspace'
1717
import { sendViewOpenedTelemetryEvent } from '../../telemetry'
1818
import { EventName } from '../../telemetry/constants'
@@ -347,18 +347,12 @@ export class ExperimentsTree
347347
}
348348

349349
private getTooltipTable(experiment: Experiment, firstThreeColumns: string[]) {
350-
const data = firstThreeColumns
351-
.map(path => {
352-
const { value, splitUpPath } = getDataFromColumnPath(experiment, path)
353-
const [type] = splitUpPath
354-
const truncatedKey = truncateFromLeft(
355-
path.slice(type.length + 1) || path,
356-
30
357-
)
358-
return value === null ? '' : `| ${truncatedKey} | ${value} |\n`
359-
})
350+
const data = getDataFromColumnPaths(experiment, firstThreeColumns)
351+
.map(
352+
({ truncatedValue: value, columnPath }) =>
353+
`| ${truncateFromLeft(columnPath, 30)} | ${value} |\n`
354+
)
360355
.join('')
361-
362356
return data === '' ? undefined : getMarkdownString(`|||\n|:--|--|\n${data}`)
363357
}
364358

‎extension/src/experiments/model/util.ts‎

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -35,22 +35,31 @@ const getStringifiedValue = (value: Value): string => {
3535
return String(value)
3636
}
3737

38-
export const getDataFromColumnPath = (
38+
const getDataFromColumnPath = (
3939
experiment: Experiment,
4040
columnPath: string
41-
): { fullValue: string; splitUpPath: string[]; value: string | null } => {
41+
): {
42+
type: string
43+
value: string | number
44+
columnPath: string
45+
splitUpPath: string[]
46+
truncatedValue: string
47+
} => {
4248
const splitUpPath = splitColumnPath(columnPath)
4349
const collectedVal = get(experiment, splitUpPath)
4450
const value = collectedVal?.value || collectedVal
51+
const [type] = splitUpPath
4552

4653
return {
47-
fullValue: isLongNumber(value)
48-
? value.toString()
49-
: getStringifiedValue(value),
54+
columnPath: columnPath.slice(type.length + 1) || columnPath,
5055
splitUpPath,
51-
value:
52-
columnPath === 'Created' && value === 'undefined'
53-
? null
54-
: getStringifiedValue(value)
56+
truncatedValue: getStringifiedValue(value),
57+
type,
58+
value: isLongNumber(value) ? value : getStringifiedValue(value)
5559
}
5660
}
61+
62+
export const getDataFromColumnPaths = (
63+
experiment: Experiment,
64+
columnPaths: string[]
65+
) => columnPaths.map(path => getDataFromColumnPath(experiment, path))

‎extension/src/plots/model/util.ts‎

Lines changed: 9 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,15 @@
1-
import { getDataFromColumnPath } from '../../experiments/model/util'
1+
import { getDataFromColumnPaths } from '../../experiments/model/util'
22
import { Experiment } from '../../experiments/webview/contract'
33
import { RevisionFirstThreeColumns } from '../webview/contract'
44

55
export const getRevisionFirstThreeColumns = (
66
firstThreeColumns: string[],
77
experiment: Experiment
8-
) => {
9-
const columns: RevisionFirstThreeColumns = []
10-
for (const path of firstThreeColumns) {
11-
const { value, splitUpPath, fullValue } = getDataFromColumnPath(
12-
experiment,
13-
path
14-
)
15-
const type = splitUpPath[0]
16-
17-
if (value) {
18-
columns.push({
19-
fullValue,
20-
path: path.slice(type.length + 1) || path,
21-
type,
22-
value
23-
})
24-
}
25-
}
26-
return columns
27-
}
8+
): RevisionFirstThreeColumns =>
9+
getDataFromColumnPaths(experiment, firstThreeColumns).map(
10+
({ columnPath: path, value, type }) => ({
11+
path,
12+
type,
13+
value
14+
})
15+
)

‎extension/src/plots/webview/contract.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,7 @@ export type ComparisonPlots = {
3737

3838
export type RevisionFirstThreeColumns = Array<{
3939
path: string
40-
value: string
41-
fullValue: string
40+
value: string | number
4241
type: string
4342
}>
4443

‎extension/src/test/fixtures/plotsDiff/index.ts‎

Lines changed: 12 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -542,21 +542,18 @@ export const getRevisions = (): Revision[] => {
542542
firstThreeColumns: [
543543
{
544544
type: 'Created',
545-
fullValue: '-',
546545
path: 'Created',
547546
value: '-'
548547
},
549548
{
550549
type: ColumnType.METRICS,
551-
fullValue: '1.9293040037155151',
552550
path: 'summary.json:loss',
553-
value: '1.9293'
551+
value: 1.9293040037155151
554552
},
555553
{
556554
type: ColumnType.METRICS,
557-
fullValue: '0.4668000042438507',
558555
path: 'summary.json:accuracy',
559-
value: '0.46680'
556+
value: 0.4668000042438507
560557
}
561558
],
562559
group: undefined
@@ -566,21 +563,18 @@ export const getRevisions = (): Revision[] => {
566563
firstThreeColumns: [
567564
{
568565
type: 'Created',
569-
fullValue: formatDate(getMain().Created as string),
570566
path: 'Created',
571567
value: formatDate(getMain().Created as string)
572568
},
573569
{
574-
type: ColumnType.METRICS,
575-
fullValue: '2.048856019973755',
576570
path: 'summary.json:loss',
577-
value: '2.0489'
571+
type: ColumnType.METRICS,
572+
value: 2.048856019973755
578573
},
579574
{
580-
type: ColumnType.METRICS,
581-
fullValue: '0.3484833240509033',
582575
path: 'summary.json:accuracy',
583-
value: '0.34848'
576+
type: ColumnType.METRICS,
577+
value: 0.3484833240509033
584578
}
585579
],
586580
id: 'main',
@@ -593,21 +587,18 @@ export const getRevisions = (): Revision[] => {
593587
firstThreeColumns: [
594588
{
595589
type: 'Created',
596-
fullValue: findAndFormatCreated('exp-e7a67'),
597590
path: 'Created',
598591
value: findAndFormatCreated('exp-e7a67')
599592
},
600593
{
601594
type: ColumnType.METRICS,
602-
fullValue: '2.0205044746398926',
603595
path: 'summary.json:loss',
604-
value: '2.0205'
596+
value: 2.0205044746398926
605597
},
606598
{
607599
type: ColumnType.METRICS,
608-
fullValue: '0.3724166750907898',
609600
path: 'summary.json:accuracy',
610-
value: '0.37242'
601+
value: 0.3724166750907898
611602
}
612603
],
613604
id: 'exp-e7a67',
@@ -619,22 +610,19 @@ export const getRevisions = (): Revision[] => {
619610
fetched: true,
620611
firstThreeColumns: [
621612
{
622-
fullValue: findAndFormatCreated('test-branch'),
623613
type: 'Created',
624614
path: 'Created',
625615
value: findAndFormatCreated('test-branch')
626616
},
627617
{
628-
fullValue: '1.9293040037155151',
629618
type: ColumnType.METRICS,
630619
path: 'summary.json:loss',
631-
value: '1.9293'
620+
value: 1.9293040037155151
632621
},
633622
{
634-
fullValue: '0.4668000042438507',
635623
type: ColumnType.METRICS,
636624
path: 'summary.json:accuracy',
637-
value: '0.46680'
625+
value: 0.4668000042438507
638626
}
639627
],
640628
id: 'test-branch',
@@ -646,22 +634,19 @@ export const getRevisions = (): Revision[] => {
646634
fetched: true,
647635
firstThreeColumns: [
648636
{
649-
fullValue: findAndFormatCreated('exp-83425'),
650637
type: 'Created',
651638
path: 'Created',
652639
value: findAndFormatCreated('exp-83425')
653640
},
654641
{
655-
fullValue: '1.775016188621521',
656642
type: ColumnType.METRICS,
657643
path: 'summary.json:loss',
658-
value: '1.7750'
644+
value: 1.775016188621521
659645
},
660646
{
661-
fullValue: '0.5926499962806702',
662647
type: ColumnType.METRICS,
663648
path: 'summary.json:accuracy',
664-
value: '0.59265'
649+
value: 0.5926499962806702
665650
}
666651
],
667652
id: 'exp-83425',

0 commit comments

Comments
 (0)