Skip to content

Commit

Permalink
Fix "plot already exists" logic
Browse files Browse the repository at this point in the history
  • Loading branch information
julieg18 committed Mar 20, 2023
1 parent 9630fdd commit 40d0010
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 15 deletions.
2 changes: 1 addition & 1 deletion extension/src/plots/model/custom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export const doesCustomPlotAlreadyExist = (
return value.param === param && value.metric === metric
})

const removeColumnTypeFromPath = (
export const removeColumnTypeFromPath = (
columnPath: string,
type: string,
fileSep: string
Expand Down
22 changes: 11 additions & 11 deletions extension/src/plots/model/quickPick.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ describe('pickMetricAndParam', () => {
{
hasChildren: false,
label: 'accuracy',
path: 'summary.json:accuracy',
path: 'metrics:summary.json:accuracy',
type: ColumnType.METRICS
}
])
Expand All @@ -169,7 +169,7 @@ describe('pickMetricAndParam', () => {
{
hasChildren: false,
label: 'accuracy',
path: 'summary.json:accuracy',
path: 'metrics:summary.json:accuracy',
type: ColumnType.METRICS
}
])
Expand All @@ -183,7 +183,7 @@ describe('pickMetricAndParam', () => {
}
const expectedParam = {
label: 'epochs',
path: 'summary.json:loss-params.yaml:epochs'
path: 'params:params.yaml:epochs'
}
mockedQuickPickValue
.mockResolvedValueOnce(expectedMetric)
Expand All @@ -200,13 +200,13 @@ describe('pickMetricAndParam', () => {
{
hasChildren: false,
label: 'accuracy',
path: 'summary.json:accuracy',
path: 'metrics:summary.json:accuracy',
type: ColumnType.METRICS
}
])
expect(metricAndParam).toStrictEqual({
metric: expectedMetric.path,
param: expectedParam.path
metric: 'summary.json:loss',
param: 'params.yaml:epochs'
})
})
})
Expand All @@ -232,7 +232,7 @@ describe('pickMetric', () => {
{
hasChildren: false,
label: 'accuracy',
path: 'summary.json:accuracy',
path: 'metrics:summary.json:accuracy',
type: ColumnType.METRICS
}
])
Expand All @@ -250,12 +250,12 @@ describe('pickMetric', () => {
{
hasChildren: false,
label: 'accuracy',
path: 'summary.json:accuracy',
path: 'metrics:summary.json:accuracy',
type: ColumnType.METRICS
}
])

expect(metric).toStrictEqual(expectedMetric.path)
expect(metric).toStrictEqual('summary.json:loss')
expect(mockedQuickPickValue).toHaveBeenCalledTimes(1)
expect(mockedQuickPickValue).toHaveBeenCalledWith(
[
Expand All @@ -265,9 +265,9 @@ describe('pickMetric', () => {
value: { label: 'loss', path: 'metrics:summary.json:loss' }
},
{
description: 'summary.json:accuracy',
description: 'metrics:summary.json:accuracy',
label: 'accuracy',
value: { label: 'accuracy', path: 'summary.json:accuracy' }
value: { label: 'accuracy', path: 'metrics:summary.json:accuracy' }
}
],
{ title: Title.SELECT_METRIC_CUSTOM_PLOT }
Expand Down
23 changes: 20 additions & 3 deletions extension/src/plots/model/quickPick.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ import { getCustomPlotId } from './collect'
import {
getFullValuePath,
CustomPlotsOrderValue,
isCheckpointValue
isCheckpointValue,
removeColumnTypeFromPath
} from './custom'
import {
FILE_SEPARATOR,
Expand Down Expand Up @@ -126,7 +127,19 @@ export const pickMetricAndParam = async (columns: Column[]) => {
if (!param) {
return
}
return { metric: metric.path, param: param.path }

return {
metric: removeColumnTypeFromPath(
metric.path,
ColumnType.METRICS,
FILE_SEPARATOR
),
param: removeColumnTypeFromPath(
param.path,
ColumnType.PARAMS,
FILE_SEPARATOR
)
}
}

export const pickMetric = async (columns: Column[]) => {
Expand All @@ -144,5 +157,9 @@ export const pickMetric = async (columns: Column[]) => {
return
}

return metric.path
return removeColumnTypeFromPath(
metric.path,
ColumnType.METRICS,
FILE_SEPARATOR
)
}

0 comments on commit 40d0010

Please sign in to comment.