Skip to content

Commit 9cf6e21

Browse files
authored
Make the plot sizes use numbers underneath (#2563)
* test * Make the plot sizes numerical underneath * Reset file * Fix comparison table stories * Fix large plots * Fix multiview sizes * Fix virtualized plots
1 parent 0fb7275 commit 9cf6e21

File tree

30 files changed

+228
-300
lines changed

30 files changed

+228
-300
lines changed

extension/src/plots/model/collect.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,7 @@ import {
1111
Plot,
1212
TemplatePlotEntry,
1313
TemplatePlotSection,
14-
PlotsType,
15-
PlotSize
14+
PlotsType
1615
} from '../webview/contract'
1716
import {
1817
ExperimentFieldsOrError,
@@ -599,7 +598,7 @@ const collectTemplateGroup = (
599598
selectedRevisions: string[],
600599
templates: TemplateAccumulator,
601600
revisionData: RevisionData,
602-
size: PlotSize,
601+
size: number,
603602
revisionColors: ColorScale | undefined,
604603
multiSourceEncoding: MultiSourceEncoding
605604
): TemplatePlotEntry[] => {
@@ -640,7 +639,7 @@ export const collectSelectedTemplatePlots = (
640639
selectedRevisions: string[],
641640
templates: TemplateAccumulator,
642641
revisionData: RevisionData,
643-
size: PlotSize,
642+
size: number,
644643
revisionColors: ColorScale | undefined,
645644
multiSourceEncoding: MultiSourceEncoding
646645
): TemplatePlotSection[] | undefined => {

extension/src/plots/model/index.test.ts

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { PlotsModel } from '.'
22
import {
33
DEFAULT_SECTION_COLLAPSED,
44
DEFAULT_SECTION_SIZES,
5-
PlotSize,
5+
PlotSizeNumber,
66
Section
77
} from '../webview/contract'
88
import { buildMockMemento } from '../../test/util'
@@ -64,25 +64,28 @@ describe('plotsModel', () => {
6464

6565
it('should change the plotSize when calling setPlotSize', () => {
6666
expect(model.getPlotSize(Section.CHECKPOINT_PLOTS)).toStrictEqual(
67-
PlotSize.REGULAR
67+
PlotSizeNumber.REGULAR
6868
)
6969

70-
model.setPlotSize(Section.CHECKPOINT_PLOTS, PlotSize.LARGE)
70+
model.setPlotSize(Section.CHECKPOINT_PLOTS, PlotSizeNumber.LARGE)
7171

7272
expect(model.getPlotSize(Section.CHECKPOINT_PLOTS)).toStrictEqual(
73-
PlotSize.LARGE
73+
PlotSizeNumber.LARGE
7474
)
7575
})
7676

7777
it('should update the persisted plot size when calling setPlotSize', () => {
7878
const mementoUpdateSpy = jest.spyOn(memento, 'update')
7979

80-
model.setPlotSize(Section.CHECKPOINT_PLOTS, PlotSize.SMALL)
80+
model.setPlotSize(Section.CHECKPOINT_PLOTS, PlotSizeNumber.REGULAR)
8181

8282
expect(mementoUpdateSpy).toHaveBeenCalledTimes(1)
8383
expect(mementoUpdateSpy).toHaveBeenCalledWith(
8484
PersistenceKey.PLOT_SIZES + exampleDvcRoot,
85-
{ ...DEFAULT_SECTION_SIZES, [Section.CHECKPOINT_PLOTS]: PlotSize.SMALL }
85+
{
86+
...DEFAULT_SECTION_SIZES,
87+
[Section.CHECKPOINT_PLOTS]: PlotSizeNumber.REGULAR
88+
}
8689
)
8790
})
8891

extension/src/plots/model/index.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,9 @@ import {
2121
ComparisonRevisionData,
2222
DEFAULT_SECTION_COLLAPSED,
2323
DEFAULT_SECTION_SIZES,
24-
PlotSize,
2524
Section,
26-
SectionCollapsed
25+
SectionCollapsed,
26+
PlotSizeNumber
2727
} from '../webview/contract'
2828
import { ExperimentsOutput, PlotsOutput } from '../../cli/dvc/contract'
2929
import { Experiments } from '../../experiments'
@@ -43,7 +43,7 @@ import {
4343
export class PlotsModel extends ModelWithPersistence {
4444
private readonly experiments: Experiments
4545

46-
private plotSizes: Record<Section, PlotSize>
46+
private plotSizes: Record<Section, number>
4747
private sectionCollapsed: SectionCollapsed
4848
private branchRevisions: Record<string, string> = {}
4949
private workspaceRunningCheckpoint: string | undefined
@@ -281,13 +281,13 @@ export class PlotsModel extends ModelWithPersistence {
281281
this.persist(PersistenceKey.PLOT_METRIC_ORDER, this.metricOrder)
282282
}
283283

284-
public setPlotSize(section: Section, size: PlotSize) {
284+
public setPlotSize(section: Section, size: number) {
285285
this.plotSizes[section] = size
286286
this.persist(PersistenceKey.PLOT_SIZES, this.plotSizes)
287287
}
288288

289289
public getPlotSize(section: Section) {
290-
return this.plotSizes[section]
290+
return this.plotSizes[section] || PlotSizeNumber.REGULAR
291291
}
292292

293293
public setSectionCollapsed(newState: Partial<SectionCollapsed>) {

extension/src/plots/vega/util.test.ts

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import scatterTemplate from '../../test/fixtures/plotsDiff/templates/scatter'
1818
import smoothTemplate from '../../test/fixtures/plotsDiff/templates/smooth'
1919
import multiSourceTemplate from '../../test/fixtures/plotsDiff/templates/multiSource'
2020
import { copyOriginalColors } from '../../experiments/model/status/colors'
21-
import { PlotSize } from '../webview/contract'
21+
import { PlotSizeNumber } from '../webview/contract'
2222

2323
describe('isMultiViewPlot', () => {
2424
it('should recognize the confusion matrix template as a multi view plot', () => {
@@ -83,7 +83,7 @@ describe('getColorScale', () => {
8383

8484
describe('extendVegaSpec', () => {
8585
it('should not add encoding if no color scale is provided', () => {
86-
const extendedSpec = extendVegaSpec(linearTemplate, PlotSize.REGULAR)
86+
const extendedSpec = extendVegaSpec(linearTemplate, PlotSizeNumber.REGULAR)
8787
expect(extendedSpec.encoding).toBeUndefined()
8888
})
8989

@@ -92,9 +92,13 @@ describe('extendVegaSpec', () => {
9292
domain: ['workspace', 'main'],
9393
range: copyOriginalColors().slice(0, 2)
9494
}
95-
const extendedSpec = extendVegaSpec(linearTemplate, PlotSize.REGULAR, {
96-
color: colorScale
97-
})
95+
const extendedSpec = extendVegaSpec(
96+
linearTemplate,
97+
PlotSizeNumber.REGULAR,
98+
{
99+
color: colorScale
100+
}
101+
)
98102

99103
expect(extendedSpec).not.toStrictEqual(defaultTemplate)
100104
expect(extendedSpec.encoding.color).toStrictEqual({
@@ -135,7 +139,7 @@ describe('extendVegaSpec', () => {
135139

136140
it('should truncate all titles from the left to 50 characters for large plots', () => {
137141
const spec = withLongTemplatePlotTitle()
138-
const updatedSpec = extendVegaSpec(spec, PlotSize.LARGE)
142+
const updatedSpec = extendVegaSpec(spec, 500)
139143

140144
const truncatedTitle = '…-many-many-characters-at-least-seventy-characters'
141145
const truncatedHorizontalTitle =
@@ -161,7 +165,7 @@ describe('extendVegaSpec', () => {
161165

162166
it('should truncate all titles from the left to 50 characters for regular plots', () => {
163167
const spec = withLongTemplatePlotTitle()
164-
const updatedSpec = extendVegaSpec(spec, PlotSize.REGULAR)
168+
const updatedSpec = extendVegaSpec(spec, PlotSizeNumber.REGULAR)
165169

166170
const truncatedTitle = '…-many-many-characters-at-least-seventy-characters'
167171
const truncatedHorizontalTitle =
@@ -187,7 +191,7 @@ describe('extendVegaSpec', () => {
187191

188192
it('should truncate all titles from the left to 30 characters for small plots', () => {
189193
const spec = withLongTemplatePlotTitle()
190-
const updatedSpec = extendVegaSpec(spec, PlotSize.SMALL)
194+
const updatedSpec = extendVegaSpec(spec, 300)
191195

192196
const truncatedTitle = '…s-at-least-seventy-characters'
193197
const truncatedHorizontalTitle = '…at-least-seventy-characters-x'
@@ -217,7 +221,7 @@ describe('extendVegaSpec', () => {
217221
text: repeatedTitle
218222
})
219223

220-
const updatedSpec = extendVegaSpec(spec, PlotSize.SMALL)
224+
const updatedSpec = extendVegaSpec(spec, 300)
221225

222226
const truncatedTitle = '…ghijklmnopqrstuvwyz1234567890'
223227

@@ -234,7 +238,7 @@ describe('extendVegaSpec', () => {
234238
const repeatedTitle = 'abcdefghijklmnopqrstuvwyz1234567890'
235239
const spec = withLongTemplatePlotTitle([repeatedTitle, repeatedTitle])
236240

237-
const updatedSpec = extendVegaSpec(spec, PlotSize.SMALL)
241+
const updatedSpec = extendVegaSpec(spec, 300)
238242

239243
const truncatedTitle = '…ghijklmnopqrstuvwyz1234567890'
240244

@@ -254,7 +258,7 @@ describe('extendVegaSpec', () => {
254258
text: [repeatedTitle, repeatedTitle]
255259
})
256260

257-
const updatedSpec = extendVegaSpec(spec, PlotSize.SMALL)
261+
const updatedSpec = extendVegaSpec(spec, 300)
258262

259263
const truncatedTitle = '…ghijklmnopqrstuvwyz1234567890'
260264

@@ -268,7 +272,7 @@ describe('extendVegaSpec', () => {
268272
})
269273

270274
it('should update the multi-source template to remove erroneous shape encoding from the vertical line displayed on hover', () => {
271-
const updatedSpec = extendVegaSpec(multiSourceTemplate, PlotSize.LARGE, {
275+
const updatedSpec = extendVegaSpec(multiSourceTemplate, 500, {
272276
color: { domain: [], range: [] },
273277
shape: {
274278
field: 'field',

extension/src/plots/vega/util.ts

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import {
2020
} from 'vega-lite/build/src/spec/repeat'
2121
import { TopLevelUnitSpec } from 'vega-lite/build/src/spec/unit'
2222
import isEqual from 'lodash.isequal'
23-
import { ColorScale, PlotSize, Revision } from '../webview/contract'
23+
import { ColorScale, PlotSizeNumber, Revision } from '../webview/contract'
2424
import { ShapeEncoding, StrokeDashEncoding } from '../multiSource/constants'
2525

2626
const COMMIT_FIELD = 'rev'
@@ -214,9 +214,9 @@ const truncateTitleAsArrayOrString = (title: Text, size: number) => {
214214
}
215215

216216
const TitleLimit = {
217-
[PlotSize.LARGE]: 50,
218-
[PlotSize.REGULAR]: 50,
219-
[PlotSize.SMALL]: 30
217+
[PlotSizeNumber.LARGE]: 50,
218+
[PlotSizeNumber.REGULAR]: 50,
219+
[PlotSizeNumber.SMALL]: 30
220220
}
221221

222222
const truncateTitlePart = (
@@ -252,15 +252,15 @@ const truncateTitle = (
252252
return titleCopy
253253
}
254254

255-
export const truncateVerticalTitle = (title: Text | Title, size: PlotSize) =>
256-
truncateTitle(title, TitleLimit[size] * 0.75)
255+
export const truncateVerticalTitle = (title: Text | Title, size: number) =>
256+
truncateTitle(title, TitleLimit[size as keyof typeof TitleLimit] * 0.75)
257257

258258
const isEndValue = (valueType: string) =>
259259
['string', 'number', 'boolean'].includes(valueType)
260260

261261
export const truncateTitles = (
262262
spec: TopLevelSpec,
263-
size: PlotSize,
263+
size: number,
264264
vertical?: boolean
265265
// eslint-disable-next-line sonarjs/cognitive-complexity
266266
) => {
@@ -281,7 +281,7 @@ export const truncateTitles = (
281281
const title = value as unknown as Title
282282
specCopy[key] = vertical
283283
? truncateVerticalTitle(title, size)
284-
: truncateTitle(title, TitleLimit[size])
284+
: truncateTitle(title, TitleLimit[size as keyof typeof TitleLimit])
285285
} else if (isEndValue(valueType)) {
286286
specCopy[key] = value
287287
} else if (Array.isArray(value)) {
@@ -299,7 +299,7 @@ export const truncateTitles = (
299299

300300
export const extendVegaSpec = (
301301
spec: TopLevelSpec,
302-
size: PlotSize,
302+
size: number,
303303
encoding?: {
304304
color?: ColorScale
305305
strokeDash?: StrokeDashEncoding

extension/src/plots/webview/contract.ts

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,22 @@
11
import { VisualizationSpec } from 'react-vega'
22
import { Color } from '../../experiments/model/status/colors'
33

4-
export const PlotSize = {
5-
LARGE: 'LARGE',
6-
REGULAR: 'REGULAR',
7-
SMALL: 'SMALL'
4+
export const PlotSizeNumber = {
5+
LARGE: 500,
6+
REGULAR: 400,
7+
SMALL: 300
88
}
99

10-
type PlotSizeKeys = keyof typeof PlotSize
11-
export type PlotSize = typeof PlotSize[PlotSizeKeys]
12-
1310
export enum Section {
1411
CHECKPOINT_PLOTS = 'checkpoint-plots',
1512
TEMPLATE_PLOTS = 'template-plots',
1613
COMPARISON_TABLE = 'comparison-table'
1714
}
1815

1916
export const DEFAULT_SECTION_SIZES = {
20-
[Section.CHECKPOINT_PLOTS]: PlotSize.REGULAR,
21-
[Section.TEMPLATE_PLOTS]: PlotSize.REGULAR,
22-
[Section.COMPARISON_TABLE]: PlotSize.REGULAR
17+
[Section.CHECKPOINT_PLOTS]: PlotSizeNumber.REGULAR,
18+
[Section.TEMPLATE_PLOTS]: PlotSizeNumber.REGULAR,
19+
[Section.COMPARISON_TABLE]: PlotSizeNumber.REGULAR
2320
}
2421

2522
export const DEFAULT_SECTION_COLLAPSED = {
@@ -46,7 +43,7 @@ export type Revision = {
4643

4744
export interface PlotsComparisonData {
4845
plots: ComparisonPlots
49-
size: PlotSize
46+
size: number
5047
}
5148

5249
export type CheckpointPlotValues = {
@@ -67,7 +64,7 @@ export type CheckpointPlotData = CheckpointPlot & { title: string }
6764
export type CheckpointPlotsData = {
6865
plots: CheckpointPlotData[]
6966
colors: ColorScale
70-
size: PlotSize
67+
size: number
7168
selectedMetrics?: string[]
7269
}
7370

@@ -115,7 +112,7 @@ export type TemplatePlotSection = {
115112

116113
export interface TemplatePlotsData {
117114
plots: TemplatePlotSection[]
118-
size: PlotSize
115+
size: number
119116
}
120117

121118
export type ComparisonPlot = {

extension/src/plots/webview/messages.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ import {
33
ComparisonPlot,
44
ComparisonRevisionData,
55
PlotsData as TPlotsData,
6-
PlotSize,
76
Section,
87
SectionCollapsed
98
} from './contract'
@@ -102,7 +101,7 @@ export class WebviewMessages {
102101
this.sendCheckpointPlotsAndEvent(EventName.VIEWS_PLOTS_METRICS_SELECTED)
103102
}
104103

105-
private setPlotSize(section: Section, size: PlotSize) {
104+
private setPlotSize(section: Section, size: number) {
106105
this.plots.setPlotSize(section, size)
107106
sendTelemetryEvent(
108107
EventName.VIEWS_PLOTS_SECTION_RESIZED,

extension/src/telemetry/constants.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { ViewColumn } from 'vscode'
22
import { WorkspaceScale } from './collect'
33
import { RegisteredCliCommands, RegisteredCommands } from '../commands/external'
44
import { SortDefinition } from '../experiments/model/sortBy'
5-
import { PlotSize, Section, SectionCollapsed } from '../plots/webview/contract'
5+
import { Section, SectionCollapsed } from '../plots/webview/contract'
66

77
export const APPLICATION_INSIGHTS_KEY = '46e8e554-d50a-471a-a53b-4af2b1cd6594'
88
export const EXTENSION_ID = 'iterative.dvc'
@@ -236,7 +236,7 @@ export interface IEventNamePropertyMapping {
236236
[EventName.VIEWS_PLOTS_METRICS_SELECTED]: undefined
237237
[EventName.VIEWS_PLOTS_REVISIONS_REORDERED]: undefined
238238
[EventName.VIEWS_PLOTS_COMPARISON_ROWS_REORDERED]: undefined
239-
[EventName.VIEWS_PLOTS_SECTION_RESIZED]: { section: Section; size: PlotSize }
239+
[EventName.VIEWS_PLOTS_SECTION_RESIZED]: { section: Section; size: number }
240240
[EventName.VIEWS_PLOTS_SECTION_TOGGLE]: Partial<SectionCollapsed>
241241
[EventName.VIEWS_PLOTS_SELECT_EXPERIMENTS]: undefined
242242
[EventName.VIEWS_PLOTS_SELECT_PLOTS]: undefined

extension/src/test/fixtures/expShow/checkpointPlots.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
import { copyOriginalColors } from '../../../experiments/model/status/colors'
2-
import { CheckpointPlotsData, PlotSize } from '../../../plots/webview/contract'
2+
import {
3+
CheckpointPlotsData,
4+
PlotSizeNumber
5+
} from '../../../plots/webview/contract'
36

47
const colors = copyOriginalColors()
58

@@ -88,7 +91,7 @@ const data: CheckpointPlotsData = {
8891
'summary.json:val_loss',
8992
'summary.json:val_accuracy'
9093
],
91-
size: PlotSize.REGULAR
94+
size: PlotSizeNumber.REGULAR
9295
}
9396

9497
export default data

0 commit comments

Comments
 (0)