Skip to content

Commit f05ffa0

Browse files
julieg18julieg18
authored
Add Multiple Y Field Selection to Plot Wizard (#4787)
1 parent 2d408ac commit f05ffa0

File tree

5 files changed

+198
-57
lines changed

5 files changed

+198
-57
lines changed

‎extension/src/fileSystem/index.test.ts

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -625,8 +625,8 @@ describe('addPlotToDvcYamlFile', () => {
625625
addPlotToDvcYamlFile('/', {
626626
template: 'simple',
627627
title: 'Simple Plot',
628-
x: { file: 'data.json', key: 'epochs' },
629-
y: { file: 'data.json', key: 'accuracy' }
628+
x: { 'data.json': 'epochs' },
629+
y: { 'data.json': 'accuracy' }
630630
})
631631

632632
expect(mockedWriteFileSync).toHaveBeenCalledWith(
@@ -654,8 +654,8 @@ describe('addPlotToDvcYamlFile', () => {
654654
addPlotToDvcYamlFile('/', {
655655
template: 'simple',
656656
title: 'simple_plot',
657-
x: { file: 'data.json', key: 'epochs' },
658-
y: { file: 'acc.json', key: 'accuracy' }
657+
x: { 'data.json': 'epochs' },
658+
y: { 'acc.json': 'accuracy' }
659659
})
660660

661661
expect(mockedWriteFileSync).toHaveBeenCalledWith(
@@ -673,8 +673,8 @@ describe('addPlotToDvcYamlFile', () => {
673673
addPlotToDvcYamlFile('/', {
674674
template: 'simple',
675675
title: 'Simple Plot',
676-
x: { file: 'data.json', key: 'epochs' },
677-
y: { file: 'data.json', key: 'accuracy' }
676+
x: { 'data.json': 'epochs' },
677+
y: { 'data.json': 'accuracy' }
678678
})
679679

680680
mockDvcYamlContent.splice(7, 0, ...mockPlotYamlContent)
@@ -696,8 +696,8 @@ describe('addPlotToDvcYamlFile', () => {
696696
addPlotToDvcYamlFile('/', {
697697
template: 'simple',
698698
title: 'Simple Plot',
699-
x: { file: 'data.json', key: 'epochs' },
700-
y: { file: 'data.json', key: 'accuracy' }
699+
x: { 'data.json': 'epochs' },
700+
y: { 'data.json': 'accuracy' }
701701
})
702702

703703
expect(mockedWriteFileSync).toHaveBeenCalledWith(
@@ -734,8 +734,8 @@ describe('addPlotToDvcYamlFile', () => {
734734
addPlotToDvcYamlFile('/', {
735735
template: 'simple',
736736
title: 'simple_plot',
737-
x: { file: 'data.json', key: 'epochs' },
738-
y: { file: 'data.json', key: 'accuracy' }
737+
x: { 'data.json': 'epochs' },
738+
y: { 'data.json': 'accuracy' }
739739
})
740740

741741
expect(mockedWriteFileSync).toHaveBeenCalledWith(

‎extension/src/fileSystem/index.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -216,11 +216,16 @@ const loadYamlAsDoc = (
216216

217217
const getPlotYamlObj = (plot: PlotConfigData) => {
218218
const { x, y, template, title } = plot
219+
220+
const yFiles = Object.keys(y)
221+
const [xFile, xKey] = Object.entries(x)[0]
222+
const oneFileUsed = yFiles.length === 1 && yFiles[0] === xFile
223+
219224
return {
220225
[title]: {
221226
template,
222-
x: x.file === y.file ? x.key : { [x.file]: x.key },
223-
y: { [y.file]: y.key }
227+
x: oneFileUsed ? xKey : x,
228+
y
224229
}
225230
}
226231
}

‎extension/src/pipeline/quickPick.test.ts

Lines changed: 117 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,11 @@ import { QuickPickItemKind } from 'vscode'
22
import { pickPlotConfiguration } from './quickPick'
33
import { getInput } from '../vscode/inputBox'
44
import { pickFiles } from '../vscode/resourcePicker'
5-
import { quickPickOne, quickPickValue } from '../vscode/quickPick'
5+
import {
6+
quickPickOne,
7+
quickPickValue,
8+
quickPickManyValues
9+
} from '../vscode/quickPick'
610
import { getFileExtension, loadDataFiles } from '../fileSystem'
711
import { Title } from '../vscode/title'
812
import { Toast } from '../vscode/toast'
@@ -13,6 +17,7 @@ const mockedGetFileExt = jest.mocked(getFileExtension)
1317
const mockedGetInput = jest.mocked(getInput)
1418
const mockedQuickPickOne = jest.mocked(quickPickOne)
1519
const mockedQuickPickValue = jest.mocked(quickPickValue)
20+
const mockedQuickPickValues = jest.mocked(quickPickManyValues)
1621
const mockedToast = jest.mocked(Toast)
1722
const mockedShowError = jest.fn()
1823
mockedToast.showError = mockedShowError
@@ -259,9 +264,13 @@ describe('pickPlotConfiguration', () => {
259264
{ data: mockValidData, file: '/file.json' }
260265
])
261266
mockedQuickPickOne.mockResolvedValueOnce('simple')
262-
mockedQuickPickValue
263-
.mockResolvedValueOnce({ file: 'file.json', key: 'actual' })
264-
.mockResolvedValueOnce({ file: 'file.json', key: 'prob' })
267+
mockedQuickPickValue.mockResolvedValueOnce({
268+
file: 'file.json',
269+
key: 'actual'
270+
})
271+
mockedQuickPickValues.mockResolvedValueOnce([
272+
{ file: 'file.json', key: 'prob' }
273+
])
265274
mockedGetInput.mockResolvedValueOnce('Simple Plot')
266275

267276
const result = await pickPlotConfiguration('/')
@@ -281,8 +290,7 @@ describe('pickPlotConfiguration', () => {
281290
],
282291
'Pick a Plot Template'
283292
)
284-
expect(mockedQuickPickValue).toHaveBeenNthCalledWith(
285-
1,
293+
expect(mockedQuickPickValue).toHaveBeenCalledWith(
286294
[
287295
{
288296
kind: QuickPickItemKind.Separator,
@@ -294,8 +302,7 @@ describe('pickPlotConfiguration', () => {
294302
],
295303
{ title: Title.SELECT_PLOT_X_METRIC }
296304
)
297-
expect(mockedQuickPickValue).toHaveBeenNthCalledWith(
298-
2,
305+
expect(mockedQuickPickValues).toHaveBeenCalledWith(
299306
[
300307
{
301308
kind: QuickPickItemKind.Separator,
@@ -315,27 +322,33 @@ describe('pickPlotConfiguration', () => {
315322
expect(result).toStrictEqual({
316323
template: 'simple',
317324
title: 'Simple Plot',
318-
x: { file: 'file.json', key: 'actual' },
319-
y: { file: 'file.json', key: 'prob' }
325+
x: { 'file.json': 'actual' },
326+
y: { 'file.json': 'prob' }
320327
})
321328
})
322329

323330
it('should let the user pick a x field and y field from multiple files', async () => {
324331
mockedPickFiles.mockResolvedValueOnce(['/file.json', '/file2.json'])
325332
mockedLoadDataFiles.mockResolvedValueOnce([
326333
{ data: mockValidData, file: '/file.json' },
327-
{ data: mockValidData, file: '/file2.json' }
334+
{
335+
data: mockValidData,
336+
file: '/file2.json'
337+
}
328338
])
329339
mockedQuickPickOne.mockResolvedValueOnce('simple')
330340
mockedGetInput.mockResolvedValueOnce('simple_plot')
331-
mockedQuickPickValue
332-
.mockResolvedValueOnce({ file: 'file.json', key: 'actual' })
333-
.mockResolvedValueOnce({ file: 'file2.json', key: 'prob' })
341+
mockedQuickPickValue.mockResolvedValueOnce({
342+
file: 'file.json',
343+
key: 'actual'
344+
})
345+
mockedQuickPickValues.mockResolvedValueOnce([
346+
{ file: 'file2.json', key: 'prob' }
347+
])
334348

335349
const result = await pickPlotConfiguration('/')
336350

337-
expect(mockedQuickPickValue).toHaveBeenNthCalledWith(
338-
1,
351+
expect(mockedQuickPickValue).toHaveBeenCalledWith(
339352
[
340353
{
341354
kind: QuickPickItemKind.Separator,
@@ -354,8 +367,7 @@ describe('pickPlotConfiguration', () => {
354367
],
355368
{ title: Title.SELECT_PLOT_X_METRIC }
356369
)
357-
expect(mockedQuickPickValue).toHaveBeenNthCalledWith(
358-
2,
370+
expect(mockedQuickPickValues).toHaveBeenCalledWith(
359371
[
360372
{
361373
kind: QuickPickItemKind.Separator,
@@ -378,8 +390,80 @@ describe('pickPlotConfiguration', () => {
378390
expect(result).toStrictEqual({
379391
template: 'simple',
380392
title: 'simple_plot',
381-
x: { file: 'file.json', key: 'actual' },
382-
y: { file: 'file2.json', key: 'prob' }
393+
x: { 'file.json': 'actual' },
394+
y: { 'file2.json': 'prob' }
395+
})
396+
})
397+
398+
it('should let the user pick multiple y fields', async () => {
399+
mockedPickFiles.mockResolvedValueOnce(['/file.json', '/file2.json'])
400+
mockedLoadDataFiles.mockResolvedValueOnce([
401+
{ data: mockValidData, file: '/file.json' },
402+
{
403+
data: mockValidData.map((value, ind) => ({ ...value, step: ind })),
404+
file: '/file2.json'
405+
}
406+
])
407+
mockedQuickPickOne.mockResolvedValueOnce('simple')
408+
mockedGetInput.mockResolvedValueOnce('simple_plot')
409+
mockedQuickPickValue.mockResolvedValueOnce({
410+
file: 'file.json',
411+
key: 'actual'
412+
})
413+
mockedQuickPickValues.mockResolvedValueOnce([
414+
{ file: 'file2.json', key: 'prob' },
415+
{ file: 'file2.json', key: 'actual' },
416+
{ file: 'file2.json', key: 'step' }
417+
])
418+
419+
const result = await pickPlotConfiguration('/')
420+
421+
expect(mockedQuickPickValue).toHaveBeenCalledWith(
422+
[
423+
{
424+
kind: QuickPickItemKind.Separator,
425+
label: 'file.json',
426+
value: undefined
427+
},
428+
{ label: 'actual', value: { file: 'file.json', key: 'actual' } },
429+
{ label: 'prob', value: { file: 'file.json', key: 'prob' } },
430+
{
431+
kind: QuickPickItemKind.Separator,
432+
label: 'file2.json',
433+
value: undefined
434+
},
435+
{ label: 'actual', value: { file: 'file2.json', key: 'actual' } },
436+
{ label: 'prob', value: { file: 'file2.json', key: 'prob' } },
437+
{ label: 'step', value: { file: 'file2.json', key: 'step' } }
438+
],
439+
{ title: Title.SELECT_PLOT_X_METRIC }
440+
)
441+
expect(mockedQuickPickValues).toHaveBeenCalledWith(
442+
[
443+
{
444+
kind: QuickPickItemKind.Separator,
445+
label: 'file.json',
446+
value: undefined
447+
},
448+
{ label: 'prob', value: { file: 'file.json', key: 'prob' } },
449+
{
450+
kind: QuickPickItemKind.Separator,
451+
label: 'file2.json',
452+
value: undefined
453+
},
454+
{ label: 'actual', value: { file: 'file2.json', key: 'actual' } },
455+
{ label: 'prob', value: { file: 'file2.json', key: 'prob' } },
456+
{ label: 'step', value: { file: 'file2.json', key: 'step' } }
457+
],
458+
{
459+
title: Title.SELECT_PLOT_Y_METRIC
460+
}
461+
)
462+
expect(result).toStrictEqual({
463+
template: 'simple',
464+
title: 'simple_plot',
465+
x: { 'file.json': 'actual' },
466+
y: { 'file2.json': ['prob', 'actual', 'step'] }
383467
})
384468
})
385469

@@ -419,13 +503,12 @@ describe('pickPlotConfiguration', () => {
419503
])
420504
mockedQuickPickOne.mockResolvedValueOnce('simple')
421505
mockedGetInput.mockResolvedValueOnce('simple_plot')
422-
mockedQuickPickValue
423-
.mockResolvedValueOnce('actual')
424-
.mockResolvedValueOnce(undefined)
506+
mockedQuickPickValue.mockResolvedValueOnce('actual')
507+
mockedQuickPickValues.mockResolvedValueOnce(undefined)
425508

426509
const result = await pickPlotConfiguration('/')
427510

428-
expect(mockedQuickPickValue).toHaveBeenCalledTimes(2)
511+
expect(mockedQuickPickValues).toHaveBeenCalledTimes(1)
429512
expect(result).toStrictEqual(undefined)
430513
})
431514

@@ -435,9 +518,16 @@ describe('pickPlotConfiguration', () => {
435518
{ data: mockValidData, file: 'file.json' }
436519
])
437520
mockedQuickPickOne.mockResolvedValueOnce('linear')
438-
mockedQuickPickValue
439-
.mockResolvedValueOnce({ file: 'file.json', key: 'actual' })
440-
.mockResolvedValueOnce({ file: 'file.json', key: 'prob' })
521+
mockedQuickPickValue.mockResolvedValueOnce({
522+
file: 'file.json',
523+
key: 'actual'
524+
})
525+
mockedQuickPickValues.mockResolvedValueOnce([
526+
{
527+
file: 'file.json',
528+
key: 'prob'
529+
}
530+
])
441531
mockedGetInput.mockResolvedValueOnce(undefined)
442532

443533
const result = await pickPlotConfiguration('/')

0 commit comments

Comments
 (0)