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.

Sign up for GitHub

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

Flatten table on sort #4685

Merged
merged 14 commits into from
Sep 29, 2023
Merged

Flatten table on sort #4685

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
e10b376
wip
julieg18 Sep 18, 2023
571524c
bit of cleanup
julieg18 Sep 19, 2023
0ed93ea
Fix tests for backend
julieg18 Sep 21, 2023
d2eeecf
Add a frontend test
julieg18 Sep 22, 2023
1949c5c
Fix filter bug and add additional tests
julieg18 Sep 22, 2023
6725b5e
Fix storybooks and add new fixture
julieg18 Sep 26, 2023
41c02b4
Fix typo
julieg18 Sep 26, 2023
0ecb32b
Merge branch 'main' into flatten-table-on-sort
julieg18 Sep 26, 2023
bc00ad8
move mock row model into separate file
julieg18 Sep 27, 2023
d2ded95
Add "Parents" Columns (#4691)
julieg18 Sep 27, 2023
9d1210d
Merge branch 'main' into flatten-table-on-sort
julieg18 Sep 27, 2023
aac8b50
Update .codeclimate.json
julieg18 Sep 29, 2023
941f541
Merge branch 'main' into flatten-table-on-sort
julieg18 Sep 29, 2023
abf9707
Merge branch 'main' into flatten-table-on-sort
julieg18 Sep 29, 2023
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
17 changes: 13 additions & 4 deletions .codeclimate.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,29 @@
"demo/",
"extension/src/test/",
"languageServer/src/test/",
"webview/src/shared/components/icons/"
"webview/src/shared/components/icons/",
"webview/src/test/mockRowModel.ts"
],
"checks": {
"file-lines": {
"config": { "threshold": 300 }
"config": {
"threshold": 300
}
},
"method-count": {
"config": {
"threshold": 30
}
},
"method-count": { "config": { "threshold": 30 } },
"method-lines": {
"config": {
"threshold": 40
}
},
"method-complexity": {
"config": { "threshold": 6 }
"config": {
"threshold": 6
}
}
}
}
14 changes: 13 additions & 1 deletion extension/src/experiments/columns/collect/order.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import { Column, ColumnType } from '../../webview/contract'
import { EXPERIMENT_COLUMN_ID } from '../constants'
import {
EXPERIMENT_COLUMN_ID,
BRANCH_COLUMN_ID,
COMMIT_COLUMN_ID
} from '../constants'

export const collectColumnOrder = async (
existingColumnOrder: string[],
Expand All @@ -25,6 +29,14 @@ export const collectColumnOrder = async (
existingColumnOrder.unshift(EXPERIMENT_COLUMN_ID)
}

if (!existingColumnOrder.includes(BRANCH_COLUMN_ID)) {
existingColumnOrder.splice(1, 0, BRANCH_COLUMN_ID)
}

if (!existingColumnOrder.includes(COMMIT_COLUMN_ID)) {
existingColumnOrder.splice(2, 0, COMMIT_COLUMN_ID)
}

return [
...existingColumnOrder,
...acc.timestamp,
Expand Down
9 changes: 9 additions & 0 deletions extension/src/experiments/columns/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,12 @@ export const timestampColumn: Column = {
}

export const EXPERIMENT_COLUMN_ID = 'id'

export const COMMIT_COLUMN_ID = 'commit'
export const BRANCH_COLUMN_ID = 'branch'

export const DEFAULT_COLUMN_IDS = [
EXPERIMENT_COLUMN_ID,
BRANCH_COLUMN_ID,
COMMIT_COLUMN_ID
]
16 changes: 14 additions & 2 deletions extension/src/experiments/columns/model.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@ import {
limitSummaryOrder
} from './util'
import { collectColumnOrder } from './collect/order'
import {
BRANCH_COLUMN_ID,
COMMIT_COLUMN_ID,
DEFAULT_COLUMN_IDS
} from './constants'
import { Column, ColumnType } from '../webview/contract'
import { ExpShowOutput } from '../../cli/dvc/contract'
import { PersistenceKey } from '../../persistence/constants'
Expand Down Expand Up @@ -104,10 +109,10 @@ export class ColumnsModel extends PathSelectionModel<Column> {

public selectFirst(firstColumns: string[]) {
const columnOrder = [
'id',
...DEFAULT_COLUMN_IDS,
...firstColumns,
...this.getColumnOrder().filter(
column => !['id', ...firstColumns].includes(column)
column => ![...DEFAULT_COLUMN_IDS, ...firstColumns].includes(column)
)
]
this.setColumnOrder(columnOrder)
Expand Down Expand Up @@ -201,6 +206,13 @@ export class ColumnsModel extends PathSelectionModel<Column> {
return this.setColumnOrderFromData(selectedColumns)
}
}

const maybeMissingDefaultColumns = [COMMIT_COLUMN_ID, BRANCH_COLUMN_ID]
for (const id of maybeMissingDefaultColumns) {
if (!this.columnOrderState.includes(id)) {
return this.setColumnOrderFromData(selectedColumns)
}
}
}

private transformAndSetChanges(data: ExpShowOutput) {
Expand Down
54 changes: 50 additions & 4 deletions extension/src/experiments/model/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -438,11 +438,19 @@ export class ExperimentsModel extends ModelWithPersistence {
}

public getRowData() {
const commitsBySha = this.applyFiltersToCommits()
const workspaceRow = {
branch: WORKSPACE_BRANCH,
...this.addDetails(this.workspace)
}
const sorts = this.getSorts()
const flattenRowData = sorts.length > 0
if (flattenRowData) {
return this.getFlattenedRowData(workspaceRow)
}

const commitsBySha: { [sha: string]: Commit } = this.applyFiltersToCommits()
const rows: Commit[] = [workspaceRow]

const rows: Commit[] = [
{ branch: WORKSPACE_BRANCH, ...this.addDetails(this.workspace) }
]
for (const { branch, sha } of this.rowOrder) {
julieg18 marked this conversation as resolved.
Show resolved Hide resolved
const commit = commitsBySha[sha]
if (!commit) {
Expand Down Expand Up @@ -829,4 +837,42 @@ export class ExperimentsModel extends ModelWithPersistence {
}
return commitsBySha
}

private applyFiltersToFlattenedCommits() {
const commitsBySha: { [sha: string]: Commit[] } = {}
const filters = this.getFilters()

for (const commit of this.commits) {
const commitWithSelectedAndStarred = this.addDetails(commit)
const experiments = this.getExperimentsByCommit(
commitWithSelectedAndStarred
)

commitsBySha[commit.sha as string] = [
commitWithSelectedAndStarred,
...(experiments || [])
].filter(exp => !!filterExperiment(filters, exp))
}

return commitsBySha
}

private getFlattenedRowData(workspaceRow: Commit): Commit[] {
const commitsBySha: { [sha: string]: Commit[] } =
this.applyFiltersToFlattenedCommits()
const rows = []

for (const { branch, sha } of this.rowOrder) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[C] Once the table is flattened we could have duplicate entries right next to each other. We will need a way to collapse these rows:

image

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only thing that will differ between records will be the branch/tags. I think we should consider concatenating the information on multiple lines into a single cell.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense! I will open a followup issue.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be fixed before moving on.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be fixed before moving on.

Oh, ok! Apologies, I misunderstood your comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in #4735

const commitsAndExps = commitsBySha[sha]
if (!commitsAndExps) {
continue
}

rows.push(
...commitsAndExps.map(commitOrExp => ({ ...commitOrExp, branch }))
)
}

return [workspaceRow, ...sortExperiments(this.getSorts(), rows)]
}
}
2 changes: 2 additions & 0 deletions extension/src/test/fixtures/expShow/base/columns.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ const nestedParamsFile = join('nested', 'params.yaml')

export const dataColumnOrder: string[] = [
'id',
'branch',
'commit',
'Created',
'metrics:summary.json:accuracy',
'metrics:summary.json:loss',
Expand Down
199 changes: 199 additions & 0 deletions extension/src/test/fixtures/expShow/sorted/columns.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,199 @@
import { join } from 'path'
import { Column, ColumnType } from '../../../../experiments/webview/contract'
import { 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,
label: 'summary.json',
parentPath: buildMetricOrParamPath(ColumnType.METRICS),
path: buildMetricOrParamPath(ColumnType.METRICS, 'summary.json')
},
{
type: ColumnType.METRICS,
hasChildren: false,
label: 'loss',
parentPath: buildMetricOrParamPath(ColumnType.METRICS, 'summary.json'),
path: buildMetricOrParamPath(ColumnType.METRICS, 'summary.json', 'loss'),
pathArray: [ColumnType.METRICS, 'summary.json', 'loss'],
firstValueType: 'number'
},
{
type: ColumnType.METRICS,
hasChildren: false,
label: 'accuracy',
parentPath: buildMetricOrParamPath(ColumnType.METRICS, 'summary.json'),
path: buildMetricOrParamPath(
ColumnType.METRICS,
'summary.json',
'accuracy'
),
pathArray: [ColumnType.METRICS, 'summary.json', 'accuracy'],
firstValueType: 'number'
},
{
type: ColumnType.METRICS,
hasChildren: false,
label: 'val_loss',
parentPath: buildMetricOrParamPath(ColumnType.METRICS, 'summary.json'),
path: buildMetricOrParamPath(
ColumnType.METRICS,
'summary.json',
'val_loss'
),
pathArray: [ColumnType.METRICS, 'summary.json', 'val_loss'],
firstValueType: 'number'
},
{
type: ColumnType.METRICS,
hasChildren: false,
label: 'val_accuracy',
parentPath: buildMetricOrParamPath(ColumnType.METRICS, 'summary.json'),
path: buildMetricOrParamPath(
ColumnType.METRICS,
'summary.json',
'val_accuracy'
),
pathArray: [ColumnType.METRICS, 'summary.json', 'val_accuracy'],
firstValueType: 'number'
},
{
type: ColumnType.PARAMS,
hasChildren: true,
label: 'params.yaml',
parentPath: ColumnType.PARAMS,
path: buildMetricOrParamPath(ColumnType.PARAMS, 'params.yaml')
},
{
type: ColumnType.PARAMS,
hasChildren: false,
label: 'code_names',
parentPath: buildMetricOrParamPath(ColumnType.PARAMS, 'params.yaml'),
path: buildMetricOrParamPath(
ColumnType.PARAMS,
'params.yaml',
'code_names'
),
pathArray: [ColumnType.PARAMS, 'params.yaml', 'code_names'],
firstValueType: 'array'
},
{
type: ColumnType.PARAMS,
hasChildren: false,
label: 'epochs',
parentPath: buildMetricOrParamPath(ColumnType.PARAMS, 'params.yaml'),
path: buildMetricOrParamPath(ColumnType.PARAMS, 'params.yaml', 'epochs'),
pathArray: [ColumnType.PARAMS, 'params.yaml', 'epochs'],
firstValueType: 'number'
},
{
type: ColumnType.PARAMS,
hasChildren: false,
label: 'learning_rate',
parentPath: buildMetricOrParamPath(ColumnType.PARAMS, 'params.yaml'),
path: buildMetricOrParamPath(
ColumnType.PARAMS,
'params.yaml',
'learning_rate'
),
pathArray: [ColumnType.PARAMS, 'params.yaml', 'learning_rate'],
firstValueType: 'number'
},
{
type: ColumnType.PARAMS,
hasChildren: false,
label: 'dvc_logs_dir',
parentPath: buildMetricOrParamPath(ColumnType.PARAMS, 'params.yaml'),
path: buildMetricOrParamPath(
ColumnType.PARAMS,
'params.yaml',
'dvc_logs_dir'
),
pathArray: [ColumnType.PARAMS, 'params.yaml', 'dvc_logs_dir'],
firstValueType: 'string'
},
{
type: ColumnType.PARAMS,
hasChildren: false,
label: 'log_file',
parentPath: buildMetricOrParamPath(ColumnType.PARAMS, 'params.yaml'),
path: buildMetricOrParamPath(ColumnType.PARAMS, 'params.yaml', 'log_file'),
pathArray: [ColumnType.PARAMS, 'params.yaml', 'log_file'],
firstValueType: 'string'
},
{
type: ColumnType.PARAMS,
hasChildren: false,
label: 'dropout',
parentPath: buildMetricOrParamPath(ColumnType.PARAMS, 'params.yaml'),
path: buildMetricOrParamPath(ColumnType.PARAMS, 'params.yaml', 'dropout'),
pathArray: [ColumnType.PARAMS, 'params.yaml', 'dropout'],
firstValueType: 'number'
},
{
type: ColumnType.PARAMS,
hasChildren: true,
label: 'process',
parentPath: buildMetricOrParamPath(ColumnType.PARAMS, 'params.yaml'),
path: buildMetricOrParamPath(ColumnType.PARAMS, 'params.yaml', 'process')
},
{
type: ColumnType.PARAMS,
hasChildren: false,
label: 'threshold',
parentPath: buildMetricOrParamPath(
ColumnType.PARAMS,
'params.yaml',
'process'
),
path: buildMetricOrParamPath(
ColumnType.PARAMS,
'params.yaml',
'process',
'threshold'
),
pathArray: [ColumnType.PARAMS, 'params.yaml', 'process', 'threshold'],
firstValueType: 'number'
},
{
type: ColumnType.PARAMS,
hasChildren: true,
label: nestedParamsFile,
parentPath: ColumnType.PARAMS,
path: buildMetricOrParamPath(ColumnType.PARAMS, nestedParamsFile)
},
{
type: ColumnType.PARAMS,
hasChildren: false,
label: 'test',
parentPath: buildMetricOrParamPath(ColumnType.PARAMS, nestedParamsFile),
path: buildMetricOrParamPath(ColumnType.PARAMS, nestedParamsFile, 'test'),
pathArray: [ColumnType.PARAMS, nestedParamsFile, 'test'],
firstValueType: 'boolean'
},
{
type: ColumnType.PARAMS,
hasChildren: false,
label: 'test_arg',
parentPath: buildMetricOrParamPath(
ColumnType.PARAMS,
'params.yaml',
'process'
),
path: buildMetricOrParamPath(
ColumnType.PARAMS,
'params.yaml',
'process',
'test_arg'
),
pathArray: [ColumnType.PARAMS, 'params.yaml', 'process', 'test_arg'],
firstValueType: 'string'
}
]

export default data
Loading
Loading