Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add plots show command (with stub) #995

Merged
merged 11 commits into from
Nov 5, 2021
1 change: 1 addition & 0 deletions extension/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -986,6 +986,7 @@
"@types/lodash.omit": "^4.5.6",
"@types/mocha": "^8.2.0",
"@types/node": "^14.14.22",
"@types/react-vega": "^7.0.0",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[F] We need this for the contract

"@types/sinon-chai": "^3.2.5",
"@types/vscode": "^1.61.0",
"chai": "^4.2.0",
Expand Down
6 changes: 5 additions & 1 deletion extension/src/cli/args.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ export enum Command {
INITIALIZE = 'init',
LIST = 'list',
MOVE = 'move',
PLOTS = 'plots',
PULL = 'pull',
PUSH = 'push',
REMOVE = 'remove',
Expand All @@ -16,6 +17,10 @@ export enum Command {
METRICS = 'metrics'
}

export enum SubCommand {
SHOW = 'show'
}

export enum Flag {
FORCE = '-f',
HELP = '-h',
Expand All @@ -29,7 +34,6 @@ export enum ExperimentSubCommand {
BRANCH = 'branch',
GARBAGE_COLLECT = 'gc',
LIST = 'list',
SHOW = 'show',
REMOVE = 'remove',
RUN = 'run'
}
Expand Down
21 changes: 21 additions & 0 deletions extension/src/cli/reader.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { createProcess } from '../processExecution'
import { getFailingMockedProcess, getMockedProcess } from '../test/util/jest'
import { getProcessEnv } from '../env'
import expShowFixture from '../test/fixtures/expShow/output'
import plotsShowFixture from '../test/fixtures/plotsShow/output'
import { Config } from '../config'

jest.mock('vscode')
Expand Down Expand Up @@ -303,6 +304,26 @@ describe('CliReader', () => {
})
})

describe('plotsShow', () => {
it('should match the expected output', async () => {
const cwd = __dirname

mockedCreateProcess.mockReturnValueOnce(
getMockedProcess(JSON.stringify(plotsShowFixture))
)

const plots = await cliReader.plotsShow(cwd)
expect(plots).toEqual(plotsShowFixture)
expect(mockedCreateProcess).not.toBeCalled()
Copy link
Member Author

@mattseddon mattseddon Nov 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[F] This second expect will fail when we remove the stub, we can then replace it with whats below

// expect(mockedCreateProcess).toBeCalledWith({
// args: ['plots', 'show', SHOW_JSON],
// cwd,
// env: mockedEnv,
// executable: 'dvc'
// })
})
})

describe('root', () => {
it('should return the root relative to the cwd', async () => {
const stdout = join('..', '..')
Expand Down
27 changes: 23 additions & 4 deletions extension/src/cli/reader.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
import isEqual from 'lodash.isequal'
import { VisualizationSpec } from 'react-vega'
import { Cli, typeCheckCommands } from '.'
import {
Args,
Command,
ExperimentFlag,
ExperimentSubCommand,
Flag,
ListFlag
ListFlag,
SubCommand
} from './args'
import { retry } from './retry'
import { trimAndSplit } from '../util/stdout'
import plotsShowFixture from '../test/fixtures/plotsShow/output'

export type PathOutput = { path: string }

Expand Down Expand Up @@ -96,12 +100,15 @@ export interface ExperimentsRepoJSONOutput {
}
}

export type PlotsOutput = Record<string, VisualizationSpec>

export const autoRegisteredCommands = {
DIFF: 'diff',
EXPERIMENT_LIST_CURRENT: 'experimentListCurrent',
EXPERIMENT_SHOW: 'experimentShow',
LIST_DVC_ONLY: 'listDvcOnly',
LIST_DVC_ONLY_RECURSIVE: 'listDvcOnlyRecursive',
PLOTS_SHOW: 'plotsShow',
STATUS: 'status'
} as const

Expand All @@ -122,10 +129,9 @@ export class CliReader extends Cli {
}

public experimentShow(cwd: string): Promise<ExperimentsRepoJSONOutput> {
return this.readProcessJson<ExperimentsRepoJSONOutput>(
return this.readShowProcessJson<ExperimentsRepoJSONOutput>(
cwd,
Command.EXPERIMENT,
ExperimentSubCommand.SHOW
Command.EXPERIMENT
)
}

Expand Down Expand Up @@ -157,6 +163,10 @@ export class CliReader extends Cli {
)
}

public plotsShow(cwd: string): Promise<PlotsOutput> {
mattseddon marked this conversation as resolved.
Show resolved Hide resolved
return this.readShowProcessJson<PlotsOutput>(cwd, Command.PLOTS)
}

public async root(cwd: string): Promise<string | undefined> {
try {
return await this.executeProcess(cwd, Command.ROOT)
Expand All @@ -172,6 +182,11 @@ export class CliReader extends Cli {
formatter: typeof trimAndSplit | typeof JSON.parse,
...args: Args
): Promise<T> {
// Stubbed until DVC ready
if (isEqual(args, ['plots', 'show', '--show-json'])) {
Copy link
Member Author

@mattseddon mattseddon Nov 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[F] This is a guess at what the command will actually be

return Promise.resolve(formatter(JSON.stringify(plotsShowFixture)))
}

const output = await retry(
() => this.executeProcess(cwd, ...args),
args.join(' ')
Expand All @@ -191,4 +206,8 @@ export class CliReader extends Cli {
Flag.SHOW_JSON
)
}

private readShowProcessJson<T>(cwd: string, command: Command): Promise<T> {
return this.readProcessJson<T>(cwd, command, SubCommand.SHOW)
}
}
8 changes: 5 additions & 3 deletions extension/src/test/fixtures/plotsShow/output.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { VisualizationSpec } from 'react-vega'

const data = {
'logs/loss.tsv': {
$schema: 'https://vega.github.io/schema/vega-lite/v5.json',
Expand Down Expand Up @@ -173,7 +175,7 @@ const data = {
]
}
]
},
} as VisualizationSpec,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[F] Have to cast otherwise we get type errors

'logs/acc.tsv': {
$schema: 'https://vega.github.io/schema/vega-lite/v5.json',
data: {
Expand Down Expand Up @@ -348,7 +350,7 @@ const data = {
]
}
]
},
} as VisualizationSpec,
'predictions.json': {
$schema: 'https://vega.github.io/schema/vega-lite/v5.json',
data: {
Expand Down Expand Up @@ -50447,7 +50449,7 @@ const data = {
}
]
}
}
} as VisualizationSpec
}

export default data
9 changes: 8 additions & 1 deletion yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3408,6 +3408,13 @@
dependencies:
"@types/react" "*"

"@types/react-vega@^7.0.0":
version "7.0.0"
resolved "https://registry.yarnpkg.com/@types/react-vega/-/react-vega-7.0.0.tgz#36e266d0c4841fc3ab8fb277a38d26d0d05cfceb"
integrity sha512-ZW+Sol0KQBi/t2tjoaEOvvKwAQF2u+ChBAfEEL6V/NXgd672XbVJ6x0pKm1fnL+YuZIG/jAZLYOfSEtaVDgs3A==
dependencies:
react-vega "*"

"@types/react@*":
version "17.0.20"
resolved "https://registry.yarnpkg.com/@types/react/-/react-17.0.20.tgz#a4284b184d47975c71658cd69e759b6bd37c3b8c"
Expand Down Expand Up @@ -12555,7 +12562,7 @@ react-textarea-autosize@^8.3.0:
use-composed-ref "^1.0.0"
use-latest "^1.0.0"

react-vega@^7.4.4:
react-vega@*, react-vega@^7.4.4:
version "7.4.4"
resolved "https://registry.yarnpkg.com/react-vega/-/react-vega-7.4.4.tgz#098bca8761e8f9b9e7ab300beef434ca18dbb843"
integrity sha512-zIQo5+iz82z0+tSHzhT0U32MkMtbzWJG4SVMEJoJlduQJvkeJCQ7qaqfjUaatOhiO8eqjE5oM81BadUAXZ5Njw==
Expand Down