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

Standardize how experiment information is displayed in plots ribbon #3725

Merged
merged 2 commits into from
Apr 21, 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
4 changes: 1 addition & 3 deletions extension/src/experiments/model/collect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -253,9 +253,7 @@ const collectExpRange = (
)

if (name) {
const description = `[${name}]`
experiment.description = description
experiment.logicalGroupName = description
experiment.description = `[${name}]`
}

collectExecutorInfo(acc, experiment, executor)
Expand Down
1 change: 0 additions & 1 deletion extension/src/experiments/webview/contract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ export type Experiment = {
executor?: Executor
id: string
label: string
logicalGroupName?: string
metrics?: MetricOrParamColumns
outs?: MetricOrParamColumns
params?: MetricOrParamColumns
Expand Down
10 changes: 5 additions & 5 deletions extension/src/plots/model/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ describe('plotsModel', () => {
)

expect(
model.getComparisonRevisions().map(({ revision }) => revision)
model.getComparisonRevisions().map(({ label }) => label)
).toStrictEqual(newOrder)
})

Expand All @@ -170,7 +170,7 @@ describe('plotsModel', () => {
model.setComparisonOrder(newOrder)

expect(
model.getComparisonRevisions().map(({ revision }) => revision)
model.getComparisonRevisions().map(({ label }) => label)
).toStrictEqual([
...newOrder,
...mockedSelectedRevisions
Expand All @@ -196,19 +196,19 @@ describe('plotsModel', () => {
model.setComparisonOrder(initialOrder)

expect(
model.getComparisonRevisions().map(({ revision }) => revision)
model.getComparisonRevisions().map(({ label }) => label)
).toStrictEqual(initialOrder)

model.setComparisonOrder()

expect(
model.getComparisonRevisions().map(({ revision }) => revision)
model.getComparisonRevisions().map(({ label }) => label)
).toStrictEqual(initialOrder.filter(revision => revision !== 'main'))

model.setComparisonOrder()

expect(
model.getComparisonRevisions().map(({ revision }) => revision)
model.getComparisonRevisions().map(({ label }) => label)
).toStrictEqual([EXPERIMENT_WORKSPACE_ID, '71f31cf', 'main'])
})
})
7 changes: 3 additions & 4 deletions extension/src/plots/model/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -202,19 +202,18 @@ export class PlotsModel extends ModelWithPersistence {
public getSelectedRevisionDetails() {
const selectedRevisions: Revision[] = []
for (const experiment of this.experiments.getSelectedRevisions()) {
const { commit, description, label, displayColor, logicalGroupName, id } =
experiment
const { commit, description, label, displayColor, id } = experiment
const revision: Revision = {
description: commit ? undefined : description,
displayColor,
errors: this.errors.getRevisionErrors(id),
fetched: this.fetchedRevs.has(id),
firstThreeColumns: getRevisionFirstThreeColumns(
this.experiments.getFirstThreeColumnOrder(),
experiment
),
group: logicalGroupName,
id,
revision: label
label
}

if (commit) {
Expand Down
4 changes: 2 additions & 2 deletions extension/src/plots/webview/contract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,9 @@ export type Revision = {
errors?: string[]
fetched: boolean
firstThreeColumns: RevisionFirstThreeColumns
group?: string
description: string | undefined
id: string
revision: string
label: string
}

export interface PlotsComparisonData {
Expand Down
4 changes: 0 additions & 4 deletions extension/src/test/fixtures/expShow/base/rows.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,6 @@ const data: Commit[] = [
executor: Executor.DVC_TASK,
id: 'exp-e7a67',
label: '4fb124a',
logicalGroupName: '[exp-e7a67]',
metrics: {
'summary.json': {
loss: 2.0205044746398926,
Expand Down Expand Up @@ -213,7 +212,6 @@ const data: Commit[] = [
description: '[test-branch]',
id: 'test-branch',
label: '42b8736',
logicalGroupName: '[test-branch]',
metrics: {
'summary.json': {
loss: 1.9293040037155151,
Expand Down Expand Up @@ -271,7 +269,6 @@ const data: Commit[] = [
executor: Executor.WORKSPACE,
id: 'exp-83425',
label: EXPERIMENT_WORKSPACE_ID,
logicalGroupName: '[exp-83425]',
metrics: {
'summary.json': {
loss: 1.775016188621521,
Expand Down Expand Up @@ -341,7 +338,6 @@ const data: Commit[] = [
error:
"unable to read: 'summary.json', JSON file structure is corrupted",
label: 'f0f9186',
logicalGroupName: '[exp-f13bca]',
metrics: {},
params: {
'params.yaml': {
Expand Down
22 changes: 11 additions & 11 deletions extension/src/test/fixtures/plotsDiff/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -536,7 +536,8 @@ export const getRevisions = (): Revision[] => {
return [
{
id: EXPERIMENT_WORKSPACE_ID,
revision: EXPERIMENT_WORKSPACE_ID,
label: EXPERIMENT_WORKSPACE_ID,
description: undefined,
displayColor: workspace,
errors: undefined,
fetched: true,
Expand All @@ -556,8 +557,7 @@ export const getRevisions = (): Revision[] => {
path: 'summary.json:accuracy',
value: 0.5926499962806702
}
],
group: undefined
]
},
{
errors: undefined,
Expand All @@ -580,9 +580,9 @@ export const getRevisions = (): Revision[] => {
}
],
id: 'main',
revision: 'main',
label: 'main',
displayColor: main,
group: undefined
description: undefined
},
{
errors: undefined,
Expand All @@ -605,9 +605,9 @@ export const getRevisions = (): Revision[] => {
}
],
id: 'exp-e7a67',
revision: '4fb124a',
label: '4fb124a',
displayColor: _4fb124a,
group: '[exp-e7a67]'
description: '[exp-e7a67]'
},
{
errors: undefined,
Expand All @@ -630,9 +630,9 @@ export const getRevisions = (): Revision[] => {
}
],
id: 'test-branch',
revision: '42b8736',
label: '42b8736',
displayColor: _42b8736,
group: '[test-branch]'
description: '[test-branch]'
},
{
errors: undefined,
Expand All @@ -655,9 +655,9 @@ export const getRevisions = (): Revision[] => {
}
],
id: 'exp-83425',
revision: EXPERIMENT_WORKSPACE_ID,
label: EXPERIMENT_WORKSPACE_ID,
displayColor: _1ba7bcd,
group: '[exp-83425]'
description: '[exp-83425]'
}
]
}
Expand Down
6 changes: 0 additions & 6 deletions extension/src/test/suite/experiments/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1587,7 +1587,6 @@ suite('Experiments Test Suite', () => {
description: '[exp-1]',
id: 'exp-1',
label: '111111',
logicalGroupName: '[exp-1]',
params: { 'params.yaml': { test: 2 } },
selected: false,
starred: false
Expand All @@ -1597,7 +1596,6 @@ suite('Experiments Test Suite', () => {
description: '[exp-2]',
id: 'exp-2',
label: '222222',
logicalGroupName: '[exp-2]',
params: { 'params.yaml': { test: 1 } },
selected: false,
starred: false
Expand All @@ -1607,7 +1605,6 @@ suite('Experiments Test Suite', () => {
description: '[exp-3]',
id: 'exp-3',
label: '333333',
logicalGroupName: '[exp-3]',
params: { 'params.yaml': { test: 3 } },
selected: false,
starred: false
Expand Down Expand Up @@ -1666,7 +1663,6 @@ suite('Experiments Test Suite', () => {
description: '[exp-2]',
id: 'exp-2',
label: '222222',
logicalGroupName: '[exp-2]',
params: { 'params.yaml': { test: 1 } },
selected: false,
starred: false
Expand All @@ -1676,7 +1672,6 @@ suite('Experiments Test Suite', () => {
description: '[exp-1]',
id: 'exp-1',
label: '111111',
logicalGroupName: '[exp-1]',
params: { 'params.yaml': { test: 2 } },
selected: false,
starred: false
Expand All @@ -1686,7 +1681,6 @@ suite('Experiments Test Suite', () => {
description: '[exp-3]',
id: 'exp-3',
label: '333333',
logicalGroupName: '[exp-3]',
params: { 'params.yaml': { test: 3 } },
selected: false,
starred: false
Expand Down
13 changes: 6 additions & 7 deletions extension/src/test/suite/experiments/model/tree.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -193,13 +193,12 @@ suite('Experiments Tree Test Suite', () => {
it('should be able to select / de-select experiments using dvc.views.experimentsTree.selectExperiments', async () => {
const { plots, plotsModel, messageSpy } = await buildPlots(disposable)

const [{ revision: selectedDisplayName, displayColor }] =
plotsModel.getSelectedRevisionDetails()
const [{ label, displayColor }] = plotsModel.getSelectedRevisionDetails()

const selectedItem = {
description: selectedDisplayName,
description: label,
label: '',
value: { id: selectedDisplayName }
value: { id: label }
}

await plots.showWebview()
Expand Down Expand Up @@ -239,12 +238,12 @@ suite('Experiments Tree Test Suite', () => {
const { selectedRevisions } = getFirstArgOfLastCall(messageSpy)

expect(
(selectedRevisions as Revision[]).map(({ displayColor, revision }) => ({
(selectedRevisions as Revision[]).map(({ displayColor, label }) => ({
displayColor,
revision
label
})),
'a message is sent with colors for the currently selected experiments'
).to.deep.equal([{ displayColor, revision: selectedDisplayName }])
).to.deep.equal([{ displayColor, label }])
}).timeout(WEBVIEW_TEST_TIMEOUT)

it('should be able to remove an experiment with dvc.views.experimentsTree.removeExperiment', async () => {
Expand Down
14 changes: 7 additions & 7 deletions webview/src/plots/components/App.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -240,12 +240,12 @@ describe('App', () => {
],
revisions: [
{
description: '[exp-a270a]',
displayColor: '#945dd6',
fetched: false,
firstThreeColumns: [],
group: '[exp-a270a]',
id: 'ad2b5ec',
revision: 'ad2b5ec'
label: 'ad2b5ec'
}
],
width: DEFAULT_NB_ITEMS_PER_ROW
Expand All @@ -256,12 +256,12 @@ describe('App', () => {
sectionCollapsed: DEFAULT_SECTION_COLLAPSED,
selectedRevisions: [
{
description: '[exp-a270a]',
displayColor: '#945dd6',
fetched: false,
firstThreeColumns: [],
group: '[exp-a270a]',
id: 'ad2b5ec',
revision: 'ad2b5ec'
label: 'ad2b5ec'
}
],
template: null
Expand Down Expand Up @@ -1903,7 +1903,7 @@ describe('App', () => {

expect(getDisplayedRevisionOrder()).toStrictEqual(
plotsRevisionsFixture.map(rev =>
rev.group ? rev.group.slice(1, -1) + rev.revision : rev.revision
rev.description ? rev.label + rev.description : rev.label
)
)
})
Expand Down Expand Up @@ -1980,7 +1980,7 @@ describe('App', () => {
renderAppWithOptionalData({
comparison: comparisonTableFixture,
selectedRevisions: plotsRevisionsFixture.map(rev => {
if (rev.revision === 'main') {
if (rev.label === 'main') {
return {
...rev,
errors: ['error']
Expand All @@ -1997,7 +1997,7 @@ describe('App', () => {
renderAppWithOptionalData({
comparison: comparisonTableFixture,
selectedRevisions: plotsRevisionsFixture.map(rev => {
if (rev.revision === 'main') {
if (rev.label === 'main') {
return {
...rev,
errors: ['error'],
Expand Down
Loading