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

Make the plot sizes use numbers underneath #2563

Merged
merged 12 commits into from
Oct 19, 2022
7 changes: 3 additions & 4 deletions extension/src/plots/model/collect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@ import {
Plot,
TemplatePlotEntry,
TemplatePlotSection,
PlotsType,
PlotSize
PlotsType
} from '../webview/contract'
import {
ExperimentFieldsOrError,
Expand Down Expand Up @@ -598,7 +597,7 @@ const collectTemplateGroup = (
selectedRevisions: string[],
templates: TemplateAccumulator,
revisionData: RevisionData,
size: PlotSize,
size: number,
revisionColors: ColorScale | undefined,
multiSourceEncoding: MultiSourceEncoding
): TemplatePlotEntry[] => {
Expand Down Expand Up @@ -639,7 +638,7 @@ export const collectSelectedTemplatePlots = (
selectedRevisions: string[],
templates: TemplateAccumulator,
revisionData: RevisionData,
size: PlotSize,
size: number,
revisionColors: ColorScale | undefined,
multiSourceEncoding: MultiSourceEncoding
): TemplatePlotSection[] | undefined => {
Expand Down
15 changes: 9 additions & 6 deletions extension/src/plots/model/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { PlotsModel } from '.'
import {
DEFAULT_SECTION_COLLAPSED,
DEFAULT_SECTION_SIZES,
PlotSize,
PlotSizeNumber,
Section
} from '../webview/contract'
import { buildMockMemento } from '../../test/util'
Expand Down Expand Up @@ -64,25 +64,28 @@ describe('plotsModel', () => {

it('should change the plotSize when calling setPlotSize', () => {
expect(model.getPlotSize(Section.CHECKPOINT_PLOTS)).toStrictEqual(
PlotSize.REGULAR
PlotSizeNumber.REGULAR
)

model.setPlotSize(Section.CHECKPOINT_PLOTS, PlotSize.LARGE)
model.setPlotSize(Section.CHECKPOINT_PLOTS, PlotSizeNumber.LARGE)

expect(model.getPlotSize(Section.CHECKPOINT_PLOTS)).toStrictEqual(
PlotSize.LARGE
PlotSizeNumber.LARGE
)
})

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

model.setPlotSize(Section.CHECKPOINT_PLOTS, PlotSize.SMALL)
model.setPlotSize(Section.CHECKPOINT_PLOTS, PlotSizeNumber.REGULAR)

expect(mementoUpdateSpy).toHaveBeenCalledTimes(1)
expect(mementoUpdateSpy).toHaveBeenCalledWith(
PersistenceKey.PLOT_SIZES + exampleDvcRoot,
{ ...DEFAULT_SECTION_SIZES, [Section.CHECKPOINT_PLOTS]: PlotSize.SMALL }
{
...DEFAULT_SECTION_SIZES,
[Section.CHECKPOINT_PLOTS]: PlotSizeNumber.REGULAR
}
)
})

Expand Down
10 changes: 5 additions & 5 deletions extension/src/plots/model/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ import {
ComparisonRevisionData,
DEFAULT_SECTION_COLLAPSED,
DEFAULT_SECTION_SIZES,
PlotSize,
Section,
SectionCollapsed
SectionCollapsed,
PlotSizeNumber
} from '../webview/contract'
import { ExperimentsOutput, PlotsOutput } from '../../cli/dvc/contract'
import { Experiments } from '../../experiments'
Expand All @@ -43,7 +43,7 @@ import {
export class PlotsModel extends ModelWithPersistence {
private readonly experiments: Experiments

private plotSizes: Record<Section, PlotSize>
private plotSizes: Record<Section, number>
private sectionCollapsed: SectionCollapsed
private branchRevisions: Record<string, string> = {}
private workspaceRunningCheckpoint: string | undefined
Expand Down Expand Up @@ -285,13 +285,13 @@ export class PlotsModel extends ModelWithPersistence {
this.persist(PersistenceKey.PLOT_METRIC_ORDER, this.metricOrder)
}

public setPlotSize(section: Section, size: PlotSize) {
public setPlotSize(section: Section, size: number) {
this.plotSizes[section] = size
this.persist(PersistenceKey.PLOT_SIZES, this.plotSizes)
}

public getPlotSize(section: Section) {
return this.plotSizes[section]
return this.plotSizes[section] || PlotSizeNumber.REGULAR
}

public setSectionCollapsed(newState: Partial<SectionCollapsed>) {
Expand Down
28 changes: 16 additions & 12 deletions extension/src/plots/vega/util.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import scatterTemplate from '../../test/fixtures/plotsDiff/templates/scatter'
import smoothTemplate from '../../test/fixtures/plotsDiff/templates/smooth'
import multiSourceTemplate from '../../test/fixtures/plotsDiff/templates/multiSource'
import { copyOriginalColors } from '../../experiments/model/status/colors'
import { PlotSize } from '../webview/contract'
import { PlotSizeNumber } from '../webview/contract'

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

describe('extendVegaSpec', () => {
it('should not add encoding if no color scale is provided', () => {
const extendedSpec = extendVegaSpec(linearTemplate, PlotSize.REGULAR)
const extendedSpec = extendVegaSpec(linearTemplate, PlotSizeNumber.REGULAR)
expect(extendedSpec.encoding).toBeUndefined()
})

Expand All @@ -92,9 +92,13 @@ describe('extendVegaSpec', () => {
domain: ['workspace', 'main'],
range: copyOriginalColors().slice(0, 2)
}
const extendedSpec = extendVegaSpec(linearTemplate, PlotSize.REGULAR, {
color: colorScale
})
const extendedSpec = extendVegaSpec(
linearTemplate,
PlotSizeNumber.REGULAR,
{
color: colorScale
}
)

expect(extendedSpec).not.toStrictEqual(defaultTemplate)
expect(extendedSpec.encoding.color).toStrictEqual({
Expand Down Expand Up @@ -135,7 +139,7 @@ describe('extendVegaSpec', () => {

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

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

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

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

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

const truncatedTitle = '…s-at-least-seventy-characters'
const truncatedHorizontalTitle = '…at-least-seventy-characters-x'
Expand Down Expand Up @@ -217,7 +221,7 @@ describe('extendVegaSpec', () => {
text: repeatedTitle
})

const updatedSpec = extendVegaSpec(spec, PlotSize.SMALL)
const updatedSpec = extendVegaSpec(spec, 300)

const truncatedTitle = '…ghijklmnopqrstuvwyz1234567890'

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

const updatedSpec = extendVegaSpec(spec, PlotSize.SMALL)
const updatedSpec = extendVegaSpec(spec, 300)

const truncatedTitle = '…ghijklmnopqrstuvwyz1234567890'

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

const updatedSpec = extendVegaSpec(spec, PlotSize.SMALL)
const updatedSpec = extendVegaSpec(spec, 300)

const truncatedTitle = '…ghijklmnopqrstuvwyz1234567890'

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

it('should update the multi-source template to remove erroneous shape encoding from the vertical line displayed on hover', () => {
const updatedSpec = extendVegaSpec(multiSourceTemplate, PlotSize.LARGE, {
const updatedSpec = extendVegaSpec(multiSourceTemplate, 500, {
color: { domain: [], range: [] },
shape: {
field: 'field',
Expand Down
18 changes: 9 additions & 9 deletions extension/src/plots/vega/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {
} from 'vega-lite/build/src/spec/repeat'
import { TopLevelUnitSpec } from 'vega-lite/build/src/spec/unit'
import isEqual from 'lodash.isequal'
import { ColorScale, PlotSize, Revision } from '../webview/contract'
import { ColorScale, PlotSizeNumber, Revision } from '../webview/contract'
import { ShapeEncoding, StrokeDashEncoding } from '../multiSource/constants'

const COMMIT_FIELD = 'rev'
Expand Down Expand Up @@ -214,9 +214,9 @@ const truncateTitleAsArrayOrString = (title: Text, size: number) => {
}

const TitleLimit = {
[PlotSize.LARGE]: 50,
[PlotSize.REGULAR]: 50,
[PlotSize.SMALL]: 30
[PlotSizeNumber.LARGE]: 50,
[PlotSizeNumber.REGULAR]: 50,
[PlotSizeNumber.SMALL]: 30
}

const truncateTitlePart = (
Expand Down Expand Up @@ -252,15 +252,15 @@ const truncateTitle = (
return titleCopy
}

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

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

export const truncateTitles = (
spec: TopLevelSpec,
size: PlotSize,
size: number,
vertical?: boolean
// eslint-disable-next-line sonarjs/cognitive-complexity
) => {
Expand All @@ -281,7 +281,7 @@ export const truncateTitles = (
const title = value as unknown as Title
specCopy[key] = vertical
? truncateVerticalTitle(title, size)
: truncateTitle(title, TitleLimit[size])
: truncateTitle(title, TitleLimit[size as keyof typeof TitleLimit])
} else if (isEndValue(valueType)) {
specCopy[key] = value
} else if (Array.isArray(value)) {
Expand All @@ -299,7 +299,7 @@ export const truncateTitles = (

export const extendVegaSpec = (
spec: TopLevelSpec,
size: PlotSize,
size: number,
encoding?: {
color?: ColorScale
strokeDash?: StrokeDashEncoding
Expand Down
23 changes: 10 additions & 13 deletions extension/src/plots/webview/contract.ts
Original file line number Diff line number Diff line change
@@ -1,25 +1,22 @@
import { VisualizationSpec } from 'react-vega'
import { Color } from '../../experiments/model/status/colors'

export const PlotSize = {
LARGE: 'LARGE',
REGULAR: 'REGULAR',
SMALL: 'SMALL'
export const PlotSizeNumber = {
LARGE: 500,
REGULAR: 400,
SMALL: 300
}

type PlotSizeKeys = keyof typeof PlotSize
export type PlotSize = typeof PlotSize[PlotSizeKeys]

export enum Section {
CHECKPOINT_PLOTS = 'checkpoint-plots',
TEMPLATE_PLOTS = 'template-plots',
COMPARISON_TABLE = 'comparison-table'
}

export const DEFAULT_SECTION_SIZES = {
[Section.CHECKPOINT_PLOTS]: PlotSize.REGULAR,
[Section.TEMPLATE_PLOTS]: PlotSize.REGULAR,
[Section.COMPARISON_TABLE]: PlotSize.REGULAR
[Section.CHECKPOINT_PLOTS]: PlotSizeNumber.REGULAR,
[Section.TEMPLATE_PLOTS]: PlotSizeNumber.REGULAR,
[Section.COMPARISON_TABLE]: PlotSizeNumber.REGULAR
}

export const DEFAULT_SECTION_COLLAPSED = {
Expand All @@ -46,7 +43,7 @@ export type Revision = {

export interface PlotsComparisonData {
plots: ComparisonPlots
size: PlotSize
size: number
}

export type CheckpointPlotValues = {
Expand All @@ -67,7 +64,7 @@ export type CheckpointPlotData = CheckpointPlot & { title: string }
export type CheckpointPlotsData = {
plots: CheckpointPlotData[]
colors: ColorScale
size: PlotSize
size: number
selectedMetrics?: string[]
}

Expand Down Expand Up @@ -115,7 +112,7 @@ export type TemplatePlotSection = {

export interface TemplatePlotsData {
plots: TemplatePlotSection[]
size: PlotSize
size: number
}

export type ComparisonPlot = {
Expand Down
3 changes: 1 addition & 2 deletions extension/src/plots/webview/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import {
ComparisonPlot,
ComparisonRevisionData,
PlotsData as TPlotsData,
PlotSize,
Section,
SectionCollapsed
} from './contract'
Expand Down Expand Up @@ -102,7 +101,7 @@ export class WebviewMessages {
this.sendCheckpointPlotsAndEvent(EventName.VIEWS_PLOTS_METRICS_SELECTED)
}

private setPlotSize(section: Section, size: PlotSize) {
private setPlotSize(section: Section, size: number) {
this.plots.setPlotSize(section, size)
sendTelemetryEvent(
EventName.VIEWS_PLOTS_SECTION_RESIZED,
Expand Down
4 changes: 2 additions & 2 deletions extension/src/telemetry/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { ViewColumn } from 'vscode'
import { WorkspaceScale } from './collect'
import { RegisteredCliCommands, RegisteredCommands } from '../commands/external'
import { SortDefinition } from '../experiments/model/sortBy'
import { PlotSize, Section, SectionCollapsed } from '../plots/webview/contract'
import { Section, SectionCollapsed } from '../plots/webview/contract'

export const APPLICATION_INSIGHTS_KEY = '46e8e554-d50a-471a-a53b-4af2b1cd6594'
export const EXTENSION_ID = 'iterative.dvc'
Expand Down Expand Up @@ -236,7 +236,7 @@ export interface IEventNamePropertyMapping {
[EventName.VIEWS_PLOTS_METRICS_SELECTED]: undefined
[EventName.VIEWS_PLOTS_REVISIONS_REORDERED]: undefined
[EventName.VIEWS_PLOTS_COMPARISON_ROWS_REORDERED]: undefined
[EventName.VIEWS_PLOTS_SECTION_RESIZED]: { section: Section; size: PlotSize }
[EventName.VIEWS_PLOTS_SECTION_RESIZED]: { section: Section; size: number }
[EventName.VIEWS_PLOTS_SECTION_TOGGLE]: Partial<SectionCollapsed>
[EventName.VIEWS_PLOTS_SELECT_EXPERIMENTS]: undefined
[EventName.VIEWS_PLOTS_SELECT_PLOTS]: undefined
Expand Down
7 changes: 5 additions & 2 deletions extension/src/test/fixtures/expShow/checkpointPlots.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import { copyOriginalColors } from '../../../experiments/model/status/colors'
import { CheckpointPlotsData, PlotSize } from '../../../plots/webview/contract'
import {
CheckpointPlotsData,
PlotSizeNumber
} from '../../../plots/webview/contract'

const colors = copyOriginalColors()

Expand Down Expand Up @@ -88,7 +91,7 @@ const data: CheckpointPlotsData = {
'summary.json:val_loss',
'summary.json:val_accuracy'
],
size: PlotSize.REGULAR
size: PlotSizeNumber.REGULAR
}

export default data
Loading