Skip to content

Commit 1ce6aac

Browse files
authored
fix: Do not show Group column for tree-tables (#1851)
- If a table has no grouped columns or only one grouped column, no point in adding an extra Group column for that situation as it is just duplicate information - Fixes #1831 - Fixes #1853 - Added unit tests, and tested with the following snippet: ```python from deephaven import read_csv, agg from deephaven.constants import NULL_INT from deephaven import empty_table _source = empty_table(100).update_view(["ID = i", "Parent = i == 0 ? NULL_INT : (int)(i / 4)"]) _insurance = read_csv("https://media.githubusercontent.com/media/deephaven/examples/main/Insurance/csv/insurance.csv") agg_list = [agg.avg(cols=["bmi", "expenses"])] small_by_list = ["region"] by_list = ["region", "age"] no_group_tree = _source.tree(id_col="ID", parent_col="Parent") no_group_agg = _insurance.rollup(aggs=agg_list, by=small_by_list, include_constituents=True) group_rollup_no_agg = _insurance.rollup(aggs=[], by=by_list, include_constituents=True) group_rollup_agg = _insurance.rollup(aggs=agg_list, by=by_list, include_constituents=True) ```
1 parent aee4047 commit 1ce6aac

13 files changed

Lines changed: 88 additions & 24 deletions

__mocks__/dh-core.js

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -841,6 +841,16 @@ class Table extends DeephavenObject {
841841
}
842842
}
843843

844+
// TreeTable and Table are different in actual implementation, but should be okay for the mock
845+
class TreeTable extends Table {
846+
constructor(props) {
847+
super(props);
848+
849+
const { groupedColumns = [] } = props;
850+
this.groupedColumns = groupedColumns;
851+
}
852+
}
853+
844854
Table.EVENT_CUSTOMCOLUMNSCHANGED = 'customcolumnschanged';
845855
Table.EVENT_FILTERCHANGED = 'filterchanged';
846856
Table.EVENT_ROWADDED = 'rowadded';
@@ -1991,8 +2001,7 @@ const dh = {
19912001
TotalsTableConfig: TotalsTableConfig,
19922002
TableViewportSubscription,
19932003
TableSubscription,
1994-
// TreeTable and Table are different in actual implementation, but should be okay for the mock
1995-
TreeTable: Table,
2004+
TreeTable: TreeTable,
19962005
Column: Column,
19972006
RangeSet,
19982007
Row: Row,

packages/iris-grid/src/IrisGridTestUtils.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,11 +98,17 @@ class IrisGridTestUtils {
9898

9999
makeTreeTable(
100100
columns = this.makeColumns(),
101+
groupedColumns: DhType.Column[] = [],
101102
size = 1000000000,
102103
sort = []
103104
): DhType.TreeTable {
104105
// eslint-disable-next-line @typescript-eslint/no-explicit-any
105-
const table = new (this.dh as any).TreeTable({ columns, size, sort });
106+
const table = new (this.dh as any).TreeTable({
107+
columns,
108+
groupedColumns,
109+
size,
110+
sort,
111+
});
106112
table.copy = jest.fn(() => Promise.resolve(table));
107113
return table;
108114
}
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
import dh from '@deephaven/jsapi-shim';
2+
import IrisGridTestUtils from './IrisGridTestUtils';
3+
import IrisGridTreeTableModel from './IrisGridTreeTableModel';
4+
5+
const irisGridTestUtils = new IrisGridTestUtils(dh);
6+
7+
describe('IrisGridTreeTableModel', () => {
8+
const expectedVirtualColumn = expect.objectContaining({
9+
name: '__DH_UI_GROUP__',
10+
displayName: 'Group',
11+
isProxy: true,
12+
});
13+
const columns = irisGridTestUtils.makeColumns();
14+
15+
test.each([
16+
[0, columns, columns],
17+
[1, columns, columns],
18+
[2, columns, [expectedVirtualColumn, ...columns]],
19+
[columns.length, columns, [expectedVirtualColumn, ...columns]],
20+
])(
21+
`create virtual columns with group length %`,
22+
(groupedLength, allColumns, expected) => {
23+
const groupedColumns = allColumns.slice(0, groupedLength);
24+
const table = irisGridTestUtils.makeTreeTable(allColumns, groupedColumns);
25+
const model = new IrisGridTreeTableModel(dh, table);
26+
expect(model.columns).toEqual(expected);
27+
}
28+
);
29+
});

packages/iris-grid/src/IrisGridTreeTableModel.ts

Lines changed: 41 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -35,27 +35,47 @@ class IrisGridTreeTableModel extends IrisGridTableModelTemplate<
3535
inputTable: DhType.InputTable | null = null
3636
) {
3737
super(dh, table, formatter, inputTable);
38-
this.virtualColumns = [
39-
{
40-
name: '__DH_UI_GROUP__',
41-
displayName: 'Group',
42-
type: TableUtils.dataType.STRING,
43-
constituentType: TableUtils.dataType.STRING,
44-
isPartitionColumn: false,
45-
isSortable: false,
46-
isProxy: true,
47-
description: 'Key column',
48-
filter: () => {
49-
throw new Error('Filter not implemented for virtual column');
50-
},
51-
sort: () => {
52-
throw new Error('Sort not implemented virtual column');
53-
},
54-
formatColor: () => {
55-
throw new Error('Color not implemented for virtual column');
56-
},
57-
} as unknown as DisplayColumn,
58-
];
38+
this.virtualColumns =
39+
table.groupedColumns.length > 1
40+
? [
41+
{
42+
name: '__DH_UI_GROUP__',
43+
displayName: 'Group',
44+
type: TableUtils.dataType.STRING,
45+
constituentType: TableUtils.dataType.STRING,
46+
isPartitionColumn: false,
47+
isSortable: false,
48+
isProxy: true,
49+
description: 'Key column',
50+
index: -1,
51+
filter: () => {
52+
throw new Error('Filter not implemented for virtual column');
53+
},
54+
sort: () => {
55+
throw new Error('Sort not implemented virtual column');
56+
},
57+
formatColor: () => {
58+
throw new Error('Color not implemented for virtual column');
59+
},
60+
get: () => {
61+
throw new Error('get not implemented for virtual column');
62+
},
63+
getFormat: () => {
64+
throw new Error('getFormat not implemented for virtual column');
65+
},
66+
formatNumber: () => {
67+
throw new Error(
68+
'formatNumber not implemented for virtual column'
69+
);
70+
},
71+
formatDate: () => {
72+
throw new Error(
73+
'formatDate not implemented for virtual column'
74+
);
75+
},
76+
},
77+
]
78+
: [];
5979
}
6080

6181
applyBufferedViewport(
-906 Bytes
Loading
-498 Bytes
Loading
-572 Bytes
Loading
-462 Bytes
Loading
-347 Bytes
Loading
-492 Bytes
Loading

0 commit comments

Comments
 (0)