Skip to content

Commit

Permalink
Make timestamp column hideable and draggable (#2239)
Browse files Browse the repository at this point in the history
Co-authored-by: mattseddon <37993418+mattseddon@users.noreply.github.com>
  • Loading branch information
julieg18 and mattseddon authored Aug 26, 2022
1 parent 41db374 commit 7c591cf
Show file tree
Hide file tree
Showing 24 changed files with 179 additions and 106 deletions.
5 changes: 4 additions & 1 deletion extension/src/experiments/columns/collect/index.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { join } from 'path'
import { collectChanges, collectColumns } from '.'
import { timestampColumn } from '../constants'
import { buildDepPath, buildMetricOrParamPath } from '../paths'
import { Column, ColumnType } from '../../webview/contract'
import outputFixture from '../../../test/fixtures/expShow/output'
Expand Down Expand Up @@ -67,7 +68,7 @@ describe('collectColumns', () => {
baseline: {}
}
})
expect(columns).toStrictEqual([])
expect(columns).toStrictEqual([timestampColumn])
})

const exampleBigNumber = 3000000000
Expand Down Expand Up @@ -335,6 +336,7 @@ describe('collectColumns', () => {
})

expect(columns.map(({ path }) => path)).toStrictEqual([
timestampColumn.path,
buildMetricOrParamPath(ColumnType.PARAMS, 'params.yaml'),
buildMetricOrParamPath(
ColumnType.PARAMS,
Expand Down Expand Up @@ -419,6 +421,7 @@ describe('collectColumns', () => {
it('should collect all params and metrics from the test fixture', () => {
expect(collectColumns(outputFixture).map(({ path }) => path)).toStrictEqual(
[
timestampColumn.path,
buildMetricOrParamPath(ColumnType.METRICS, 'summary.json'),
buildMetricOrParamPath(ColumnType.METRICS, 'summary.json', 'loss'),
buildMetricOrParamPath(ColumnType.METRICS, 'summary.json', 'accuracy'),
Expand Down
3 changes: 3 additions & 0 deletions extension/src/experiments/columns/collect/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
ExperimentsOutput
} from '../../../cli/dvc/reader'
import { standardizePath } from '../../../fileSystem/path'
import { timestampColumn } from '../constants'

const collectFromExperiment = (
acc: ColumnAccumulator,
Expand All @@ -38,6 +39,8 @@ const collectFromBranch = (
export const collectColumns = (data: ExperimentsOutput): Column[] => {
const acc: ColumnAccumulator = {}

acc.timestamp = timestampColumn

const { workspace, ...rest } = data
collectFromBranch(acc, workspace)
for (const branch of Object.values(rest)) {
Expand Down
9 changes: 9 additions & 0 deletions extension/src/experiments/columns/constants.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { ColumnType } from '../webview/contract'

export const timestampColumn = {
hasChildren: false,
label: 'Timestamp',
parentPath: ColumnType.TIMESTAMP,
path: 'Timestamp',
type: ColumnType.TIMESTAMP
}
2 changes: 2 additions & 0 deletions extension/src/experiments/columns/extract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ type Columns = {
deps: DepColumns | undefined
metrics: MetricOrParamColumns | undefined
params: MetricOrParamColumns | undefined
Timestamp: string | null | undefined
}

export const extractColumns = (
Expand All @@ -88,6 +89,7 @@ export const extractColumns = (
].join('\n')

const columns: Columns = {
Timestamp: experiment?.timestamp,
deps: extractDeps(experiment.deps, branch),
metrics: metricsData?.columns,
params: paramsData?.columns
Expand Down
3 changes: 3 additions & 0 deletions extension/src/experiments/columns/model.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { ColumnsModel } from './model'
import { appendColumnToPath, buildMetricOrParamPath } from './paths'
import { timestampColumn } from './constants'
import { buildMockMemento } from '../../test/util'
import { Status } from '../../path/selection/model'
import { PersistenceKey } from '../../persistence/constants'
Expand Down Expand Up @@ -61,6 +62,7 @@ describe('ColumnsModel', () => {
const model = new ColumnsModel(exampleDvcRoot, buildMockMemento())
await model.transformAndSet(exampleData)
expect(model.getSelected()).toStrictEqual([
timestampColumn,
{
hasChildren: true,
label: 'params.yaml',
Expand Down Expand Up @@ -93,6 +95,7 @@ describe('ColumnsModel', () => {
)
await model.transformAndSet(exampleData)
expect(model.getSelected()).toStrictEqual([
timestampColumn,
{
hasChildren: true,
label: 'params.yaml',
Expand Down
4 changes: 2 additions & 2 deletions extension/src/experiments/columns/model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,8 @@ export class ColumnsModel extends PathSelectionModel<Column> {
)
}

public hasColumns() {
return this.data.length > 0
public hasNonDefaultColumns() {
return this.data.length > 1
}

private async transformAndSetColumns(data: ExperimentsOutput) {
Expand Down
13 changes: 8 additions & 5 deletions extension/src/experiments/columns/quickPick.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { ColumnLike } from './like'
import { timestampColumn } from './constants'
import { definedAndNonEmpty } from '../../util/array'
import {
QuickPickOptionsWithTitle,
Expand All @@ -14,11 +15,13 @@ export const pickFromColumnLikes = (
return Toast.showError('There are no columns to select from.')
}
return quickPickValue<ColumnLike>(
columnLikes.map(columnLike => ({
description: columnLike.path,
label: columnLike.label,
value: columnLike
})),
columnLikes
.filter(columnLike => columnLike.path !== timestampColumn.path)
.map(columnLike => ({
description: columnLike.path,
label: columnLike.label,
value: columnLike
})),
quickPickOptions
)
}
19 changes: 18 additions & 1 deletion extension/src/experiments/columns/tree.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
buildMetricOrParamPath,
splitColumnPath
} from './paths'
import { timestampColumn } from './constants'
import columnsFixture from '../../test/fixtures/expShow/columns'
import { Resource, ResourceLocator } from '../../resourceLocator'
import { RegisteredCommands } from '../../commands/external'
Expand Down Expand Up @@ -101,7 +102,7 @@ describe('ExperimentsColumnsTree', () => {
)
})

it('should return the params and metrics if there is only a single repository and no path is provided', async () => {
it('should return the params, metrics, and timestamp if there is only a single repository and no path is provided', async () => {
const experimentsColumnsTree = new ExperimentsColumnsTree(
mockedExperiments,
mockedInternalCommands,
Expand All @@ -115,6 +116,14 @@ describe('ExperimentsColumnsTree', () => {
const children = await experimentsColumnsTree.getChildren()

expect(children).toStrictEqual([
{
collapsibleState: 0,
description: undefined,
dvcRoot: mockedDvcRoot,
iconPath: mockedSelectedCheckbox,
label: timestampColumn.label,
path: timestampColumn.path
},
{
collapsibleState: 1,
description: undefined,
Expand Down Expand Up @@ -194,6 +203,14 @@ describe('ExperimentsColumnsTree', () => {
const processPath = appendColumnToPath(paramsPath, 'process')

expect(children).toStrictEqual([
{
collapsibleState: 0,
description: undefined,
dvcRoot: mockedDvcRoot,
iconPath: mockedSelectedCheckbox,
label: timestampColumn.label,
path: timestampColumn.path
},
{
collapsibleState: 1,
description: undefined,
Expand Down
13 changes: 8 additions & 5 deletions extension/src/experiments/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Event, EventEmitter, Memento } from 'vscode'
import omit from 'lodash.omit'
import { addStarredToColumns } from './columns/like'
import { setContextForEditorTitleIcons } from './context'
import { ExperimentsModel } from './model'
Expand Down Expand Up @@ -33,7 +34,7 @@ import { pickPaths } from '../path/selection/quickPick'
import { Toast } from '../vscode/toast'

export const ExperimentsScale = {
...ColumnType,
...omit(ColumnType, 'TIMESTAMP'),
HAS_CHECKPOINTS: 'hasCheckpoints',
NO_CHECKPOINTS: 'noCheckpoints'
} as const
Expand Down Expand Up @@ -182,7 +183,9 @@ export class Experiments extends BaseRepository<TableData> {
const acc = createTypedAccumulator(ExperimentsScale)

for (const { type } of this.columns.getTerminalNodes()) {
acc[type] = acc[type] + 1
if (type in acc) {
acc[<keyof typeof acc>type] = acc[<keyof typeof acc>type] + 1
}
}
const checkpointType = this.hasCheckpoints()
? ExperimentsScale.HAS_CHECKPOINTS
Expand Down Expand Up @@ -277,7 +280,7 @@ export class Experiments extends BaseRepository<TableData> {
}

public getExperimentCount() {
if (!this.columns.hasColumns()) {
if (!this.columns.hasNonDefaultColumns()) {
return 0
}

Expand Down Expand Up @@ -349,7 +352,7 @@ export class Experiments extends BaseRepository<TableData> {
}

public getExperiments() {
if (!this.columns.hasColumns()) {
if (!this.columns.hasNonDefaultColumns()) {
return []
}

Expand All @@ -365,7 +368,7 @@ export class Experiments extends BaseRepository<TableData> {
}

public getSelectedRevisions() {
if (!this.columns.hasColumns()) {
if (!this.columns.hasNonDefaultColumns()) {
return []
}

Expand Down
5 changes: 4 additions & 1 deletion extension/src/experiments/model/collect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,11 +125,14 @@ const transformColumns = (
experimentFields: ExperimentFields,
branch?: Experiment
) => {
const { error, metrics, params, deps } = extractColumns(
const { error, metrics, params, deps, Timestamp } = extractColumns(
experimentFields,
branch
)

if (Timestamp) {
experiment.Timestamp = Timestamp
}
if (metrics) {
experiment.metrics = metrics
}
Expand Down
18 changes: 9 additions & 9 deletions extension/src/experiments/model/status/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,17 @@ describe('canSelect', () => {
describe('limitToMaxSelected', () => {
const mockedExperiments = [
{ id: '1', label: 'A' },
{ id: '2', label: 'B', timestamp: null },
{ id: '3', label: 'C', timestamp: '2022-02-20T09:10:52' },
{ id: '4', label: 'D', timestamp: '2022-02-20T09:10:53' },
{ id: '5', label: 'E', timestamp: '2022-02-20T09:10:54' },
{ id: '6', label: 'F', timestamp: '2022-02-20T09:10:55' },
{ id: '7', label: 'G', timestamp: '2022-02-20T09:10:56' },
{ id: '8', label: 'H', timestamp: '2022-02-20T09:10:57' },
{ id: '9', label: 'I', timestamp: '2022-02-20T09:10:58' }
{ Timestamp: null, id: '2', label: 'B' },
{ Timestamp: '2022-02-20T09:10:52', id: '3', label: 'C' },
{ Timestamp: '2022-02-20T09:10:53', id: '4', label: 'D' },
{ Timestamp: '2022-02-20T09:10:54', id: '5', label: 'E' },
{ Timestamp: '2022-02-20T09:10:55', id: '6', label: 'F' },
{ Timestamp: '2022-02-20T09:10:56', id: '7', label: 'G' },
{ Timestamp: '2022-02-20T09:10:57', id: '8', label: 'H' },
{ Timestamp: '2022-02-20T09:10:58', id: '9', label: 'I' }
] as Experiment[]

it('should return the first 7 selected by timestamp', () => {
it('should return the first 7 selected by Timestamp', () => {
expect(
limitToMaxSelected(mockedExperiments)
.map(({ label }) => label)
Expand Down
2 changes: 1 addition & 1 deletion extension/src/experiments/model/status/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ const getEpoch = (timestamp: string | null | undefined) =>
new Date(timestamp || 0).getTime()

const compareTimestamps = (a: Experiment, b: Experiment) =>
getEpoch(b.timestamp) - getEpoch(a.timestamp)
getEpoch(b.Timestamp) - getEpoch(a.Timestamp)

export const limitToMaxSelected = (experiments: Experiment[]) =>
experiments
Expand Down
4 changes: 3 additions & 1 deletion extension/src/experiments/webview/contract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export interface Experiment extends BaseExperimentFields {
selected?: boolean
sha?: string
starred?: boolean
Timestamp?: string
}

export interface Row extends Experiment {
Expand All @@ -45,7 +46,8 @@ export interface ColumnAggregateData {
export enum ColumnType {
METRICS = 'metrics',
PARAMS = 'params',
DEPS = 'deps'
DEPS = 'deps',
TIMESTAMP = 'timestamp'
}

export interface Column extends ColumnAggregateData {
Expand Down
2 changes: 1 addition & 1 deletion extension/src/experiments/webview/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ export class WebviewMessages {
),
filters: this.experiments.getFilterPaths(),
hasCheckpoints: this.checkpoints.hasCheckpoints(),
hasColumns: this.columns.hasColumns(),
hasColumns: this.columns.hasNonDefaultColumns(),
hasRunningExperiment: this.experiments.hasRunningExperiment(),
rows: this.experiments.getRowData(),
sorts: this.experiments.getSorts()
Expand Down
2 changes: 2 additions & 0 deletions extension/src/test/fixtures/expShow/columns.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@ import {
buildDepPath,
buildMetricOrParamPath
} from '../../../experiments/columns/paths'
import { timestampColumn } from '../../../experiments/columns/constants'

const nestedParamsFile = join('nested', 'params.yaml')

const data: Column[] = [
timestampColumn,
{
type: ColumnType.METRICS,
hasChildren: true,
Expand Down
7 changes: 4 additions & 3 deletions extension/src/test/fixtures/expShow/dataTypes.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { ExperimentsOutput } from '../../../cli/dvc/reader'
import { timestampColumn } from '../../../experiments/columns/constants'
import {
Column,
ColumnType,
Expand Down Expand Up @@ -45,6 +46,7 @@ export const dataTypesOutput: ExperimentsOutput = {
}

export const columns: Column[] = [
timestampColumn,
{
hasChildren: true,
label: 'params.yaml',
Expand Down Expand Up @@ -160,8 +162,7 @@ export const rows: Row[] = [
queued: false,
running: false,
selected: true,
starred: false,
timestamp: null
starred: false
},
{
displayColor: '#13adc7',
Expand All @@ -174,7 +175,7 @@ export const rows: Row[] = [
selected: true,
sha: '53c3851f46955fa3e2b8f6e1c52999acc8c9ea77',
starred: false,
timestamp: '2020-11-21T19:58:22'
Timestamp: '2020-11-21T19:58:22'
}
]

Expand Down
5 changes: 3 additions & 2 deletions extension/src/test/fixtures/expShow/deeplyNested.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { ExperimentsOutput } from '../../../cli/dvc/reader'
import { timestampColumn } from '../../../experiments/columns/constants'
import {
Column,
ColumnType,
Expand Down Expand Up @@ -72,6 +73,7 @@ export const deeplyNestedOutput: ExperimentsOutput = {
}

export const columns: Column[] = [
timestampColumn,
{
hasChildren: true,
label: 'params.yaml',
Expand Down Expand Up @@ -218,7 +220,6 @@ export const rows = [
{
id: 'workspace',
label: 'workspace',
timestamp: null,
queued: false,
running: false,
executor: null,
Expand Down Expand Up @@ -248,7 +249,7 @@ export const rows = [
{
id: 'main',
label: 'main',
timestamp: '2020-11-21T19:58:22',
Timestamp: '2020-11-21T19:58:22',
queued: false,
running: false,
executor: null,
Expand Down
Loading

0 comments on commit 7c591cf

Please sign in to comment.