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

Bump min DVC version to 2.52.0 (plots errors) #3477

Merged
merged 5 commits into from
Apr 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion demo
Submodule demo updated 1 files
+2 −2 requirements.txt
17 changes: 14 additions & 3 deletions extension/src/cli/dvc/contract.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Plot } from '../../plots/webview/contract'

export const MIN_CLI_VERSION = '2.30.0'
export const LATEST_TESTED_CLI_VERSION = '2.51.0'
export const MIN_CLI_VERSION = '2.52.0'
export const LATEST_TESTED_CLI_VERSION = '2.52.0'
export const MAX_CLI_VERSION = '3'

type ErrorContents = { type: string; msg: string }
Expand Down Expand Up @@ -96,8 +96,19 @@ export interface ExperimentsOutput {
}
}

export interface PlotsOutput {
export interface PlotsData {
[path: string]: Plot[]
}

type PlotError = {
name: string
rev: string
source?: string
} & ErrorContents

export interface PlotsOutput {
data: { [path: string]: Plot[] }
errors?: PlotError[]
}

export type PlotsOutputOrError = PlotsOutput | DvcError
6 changes: 4 additions & 2 deletions extension/src/plots/data/collect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,16 @@ const collectKeyData = (acc: string[], key: string, plots: Plot[]): void => {
}

export const collectFiles = (
data: PlotsOutputOrError,
output: PlotsOutputOrError,
collectedFiles: string[]
): string[] => {
const acc = [...collectedFiles]
if (isDvcError(data)) {
if (isDvcError(output)) {
return acc
}

const { data } = output

for (const [key, plots] of Object.entries(data)) {
collectKeyData(acc, key, plots)
}
Expand Down
5 changes: 3 additions & 2 deletions extension/src/plots/model/collect.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ import { Experiment } from '../../experiments/webview/contract'

const logsLossPath = join('logs', 'loss.tsv')

const logsLossPlot = (plotsDiffFixture[logsLossPath][0] || {}) as TemplatePlot
const logsLossPlot = (plotsDiffFixture.data[logsLossPath][0] ||
{}) as TemplatePlot

describe('collectCustomPlots', () => {
const defaultFuncArgs = {
Expand Down Expand Up @@ -147,7 +148,7 @@ describe('collectData', () => {

expect(_1ba7bcd_heatmap).toBeDefined()
expect(_1ba7bcd_heatmap).toStrictEqual(
plotsDiffFixture[heatmapPlot].find(({ revisions }) =>
plotsDiffFixture.data[heatmapPlot].find(({ revisions }) =>
sameContents(revisions as string[], ['1ba7bcd'])
)
)
Expand Down
6 changes: 4 additions & 2 deletions extension/src/plots/model/collect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -283,9 +283,10 @@ const collectPathData = (
}

export const collectData = (
data: PlotsOutput,
output: PlotsOutput,
cliIdToLabel: CLIRevisionIdToLabel
): DataAccumulator => {
const { data } = output
const acc = {
comparisonData: {},
revisionData: {}
Expand All @@ -312,7 +313,8 @@ const collectTemplate = (
acc[path] = template
}

export const collectTemplates = (data: PlotsOutput): TemplateAccumulator => {
export const collectTemplates = (output: PlotsOutput): TemplateAccumulator => {
const { data } = output
const acc: TemplateAccumulator = {}

for (const [path, plots] of Object.entries(data)) {
Expand Down
3 changes: 2 additions & 1 deletion extension/src/plots/multiSource/collect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,10 @@ const collectPathMultiSourceVariations = (
}

export const collectMultiSourceVariations = (
data: PlotsOutput,
output: PlotsOutput,
acc: Record<string, Record<string, unknown>[]>
) => {
const { data } = output
for (const [path, plots] of Object.entries(data)) {
collectPathMultiSourceVariations(acc, path, plots)
}
Expand Down
100 changes: 52 additions & 48 deletions extension/src/plots/paths/collect.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,21 +90,23 @@ describe('collectPath', () => {
const updatedPaths = collectPaths(
collectedPaths,
{
[remainingPath]: [
{
content: {},
datapoints: {
[EXPERIMENT_WORKSPACE_ID]: [
{
loss: '2.43323',
step: '0'
}
]
},
revisions: [EXPERIMENT_WORKSPACE_ID],
type: PlotsType.VEGA
}
]
data: {
[remainingPath]: [
{
content: {},
datapoints: {
[EXPERIMENT_WORKSPACE_ID]: [
{
loss: '2.43323',
step: '0'
}
]
},
revisions: [EXPERIMENT_WORKSPACE_ID],
type: PlotsType.VEGA
}
]
}
},
[EXPERIMENT_WORKSPACE_ID],
{}
Expand Down Expand Up @@ -138,39 +140,41 @@ describe('collectPath', () => {
it('should handle more complex paths', () => {
const revisions = [EXPERIMENT_WORKSPACE_ID]
const mockPlotsDiff = {
[join('logs', 'scalars', 'acc.tsv')]: [
{
content: {},
datapoints: { [EXPERIMENT_WORKSPACE_ID]: [{}] },
revisions,
type: PlotsType.VEGA
}
],
[join('logs', 'scalars', 'loss.tsv')]: [
{
content: {},
datapoints: { [EXPERIMENT_WORKSPACE_ID]: [{}] },
revisions,
type: PlotsType.VEGA
}
],
[join('plots', 'heatmap.png')]: [
{
revisions,
type: PlotsType.IMAGE,
url: join('plots', 'heatmap.png')
}
],
'predictions.json': [
{
content: {
facet: { field: 'rev', type: 'nominal' }
} as VisualizationSpec,
datapoints: { [EXPERIMENT_WORKSPACE_ID]: [{}] },
revisions,
type: PlotsType.VEGA
}
]
data: {
[join('logs', 'scalars', 'acc.tsv')]: [
{
content: {},
datapoints: { [EXPERIMENT_WORKSPACE_ID]: [{}] },
revisions,
type: PlotsType.VEGA
}
],
[join('logs', 'scalars', 'loss.tsv')]: [
{
content: {},
datapoints: { [EXPERIMENT_WORKSPACE_ID]: [{}] },
revisions,
type: PlotsType.VEGA
}
],
[join('plots', 'heatmap.png')]: [
{
revisions,
type: PlotsType.IMAGE,
url: join('plots', 'heatmap.png')
}
],
'predictions.json': [
{
content: {
facet: { field: 'rev', type: 'nominal' }
} as VisualizationSpec,
datapoints: { [EXPERIMENT_WORKSPACE_ID]: [{}] },
revisions,
type: PlotsType.VEGA
}
]
}
}

expect(collectPaths([], mockPlotsDiff, revisions, {})).toStrictEqual([
Expand Down
16 changes: 11 additions & 5 deletions extension/src/plots/paths/collect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@ import {
TemplatePlot,
TemplatePlotGroup
} from '../webview/contract'
import { EXPERIMENT_WORKSPACE_ID, PlotsOutput } from '../../cli/dvc/contract'
import {
EXPERIMENT_WORKSPACE_ID,
PlotsData,
PlotsOutput
} from '../../cli/dvc/contract'
import { getParent, getPath, getPathArray } from '../../fileSystem/util'
import { splitMatchedOrdered, definedAndNonEmpty } from '../../util/array'
import { isMultiViewPlot } from '../vega/util'
Expand Down Expand Up @@ -50,7 +54,7 @@ const collectType = (plots: Plot[]) => {
}

const getType = (
data: PlotsOutput,
data: PlotsData,
hasChildren: boolean,
path: string
): Set<PathType> | undefined => {
Expand Down Expand Up @@ -105,7 +109,7 @@ const collectTemplateRevisions = (
}

const collectPathRevisions = (
data: PlotsOutput,
data: PlotsData,
path: string,
cliIdToLabel: { [id: string]: string }
): Set<string> => {
Expand All @@ -125,7 +129,7 @@ const collectPathRevisions = (

const collectOrderedPath = (
acc: PlotPath[],
data: PlotsOutput,
data: PlotsData,
revisions: Set<string>,
pathArray: string[],
idx: number
Expand Down Expand Up @@ -163,12 +167,14 @@ const collectOrderedPath = (

export const collectPaths = (
existingPaths: PlotPath[],
data: PlotsOutput,
output: PlotsOutput,
fetchedRevs: string[],
cliIdToLabel: { [id: string]: string }
): PlotPath[] => {
let acc: PlotPath[] = filterWorkspaceIfFetched(existingPaths, fetchedRevs)

const { data } = output

const paths = Object.keys(data)

for (const path of paths) {
Expand Down
52 changes: 27 additions & 25 deletions extension/src/plots/paths/model.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,29 +83,31 @@ describe('PathsModel', () => {
const commitBeforePlots = '4c4318d'
const previousPlotPath = join('dvclive', 'plots', 'metrics', 'loss.tsv')
const previousPlotFixture = {
[previousPlotPath]: [
{
content: {},
datapoints: {
[commitBeforePlots]: [
{
loss: '2.29',
step: '0'
},
{
loss: '2.27',
step: '1'
},
{
loss: '2.25',
step: '2'
}
]
},
revisions: [commitBeforePlots],
type: PlotsType.VEGA
}
]
data: {
[previousPlotPath]: [
{
content: {},
datapoints: {
[commitBeforePlots]: [
{
loss: '2.29',
step: '0'
},
{
loss: '2.27',
step: '1'
},
{
loss: '2.25',
step: '2'
}
]
},
revisions: [commitBeforePlots],
type: PlotsType.VEGA
}
]
}
}

const multiViewGroup = {
Expand Down Expand Up @@ -205,7 +207,7 @@ describe('PathsModel', () => {
const model = new PathsModel(mockDvcRoot, buildMockMemento())

model.transformAndSet(
{ ...plotsDiffFixture, ...previousPlotFixture },
{ data: { ...plotsDiffFixture.data, ...previousPlotFixture.data } },
[...revisions, commitBeforePlots],
{}
)
Expand Down Expand Up @@ -243,7 +245,7 @@ describe('PathsModel', () => {
it('should move plots which do not have selected revisions to the end when a reordering occurs', () => {
const model = new PathsModel(mockDvcRoot, buildMockMemento())
model.transformAndSet(
{ ...plotsDiffFixture, ...previousPlotFixture },
{ data: { ...plotsDiffFixture.data, ...previousPlotFixture.data } },
[...revisions, commitBeforePlots],
{}
)
Expand Down
7 changes: 2 additions & 5 deletions extension/src/test/e2e/extension.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ describe('Experiments Table Webview', function () {
const webview = new ExperimentsWebview('experiments')

const epochs = 15
const finalExperimentRow = 1
const headerRows = 3
const workspaceRow = 1
const commitRows = 3
Expand Down Expand Up @@ -89,7 +88,7 @@ describe('Experiments Table Webview', function () {
async () => {
await webview.expandAllRows()
const currentRows = await webview.row$$
return currentRows.length >= initialRows + epochs + finalExperimentRow
return currentRows.length >= initialRows + epochs
},
{ interval: 5000, timeout: 180000 }
)
Expand All @@ -100,9 +99,7 @@ describe('Experiments Table Webview', function () {

const finalRows = await webview.row$$

expect(finalRows.length).toStrictEqual(
initialRows + epochs + finalExperimentRow
)
expect(finalRows.length).toStrictEqual(initialRows + epochs)
await webview.unfocus()
await closeAllEditors()
await waitForDvcToFinish()
Expand Down
10 changes: 6 additions & 4 deletions extension/src/test/fixtures/plotsDiff/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -451,12 +451,14 @@ const getImageData = (baseUrl: string, joinFunc = join) => ({
})

export const getOutput = (baseUrl: string): PlotsOutput => ({
...getImageData(baseUrl),
...basicVega,
...require('./vega').default
data: {
...getImageData(baseUrl),
...basicVega,
...require('./vega').default
}
})

export const getMinimalOutput = (): PlotsOutput => ({ ...basicVega })
export const getMinimalOutput = (): PlotsOutput => ({ data: { ...basicVega } })

export const getMultiSourceOutput = (): PlotsOutput => ({
...require('./multiSource').default
Expand Down
Loading