Skip to content

Commit

Permalink
fix: Do not show Group column for tree-tables
Browse files Browse the repository at this point in the history
- 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 deephaven#1831
- 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)
```
  • Loading branch information
mofojed committed Mar 5, 2024
1 parent fe6c779 commit e8196bb
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 27 deletions.
13 changes: 11 additions & 2 deletions __mocks__/dh-core.js
Original file line number Diff line number Diff line change
Expand Up @@ -841,6 +841,16 @@ class Table extends DeephavenObject {
}
}

// TreeTable and Table are different in actual implementation, but should be okay for the mock
class TreeTable extends Table {
constructor(props) {
super(props);

const { groupedColumns = [] } = props;
this.groupedColumns = groupedColumns;
}
}

Table.EVENT_CUSTOMCOLUMNSCHANGED = 'customcolumnschanged';
Table.EVENT_FILTERCHANGED = 'filterchanged';
Table.EVENT_ROWADDED = 'rowadded';
Expand Down Expand Up @@ -1991,8 +2001,7 @@ const dh = {
TotalsTableConfig: TotalsTableConfig,
TableViewportSubscription,
TableSubscription,
// TreeTable and Table are different in actual implementation, but should be okay for the mock
TreeTable: Table,
TreeTable: TreeTable,
Column: Column,
RangeSet,
Row: Row,
Expand Down
8 changes: 7 additions & 1 deletion packages/iris-grid/src/IrisGridTestUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,11 +110,17 @@ class IrisGridTestUtils {

makeTreeTable(
columns = this.makeColumns(),
groupedColumns: Column[] = [],
size = 1000000000,
sort = []
): TreeTable {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const table = new (this.dh as any).TreeTable({ columns, size, sort });
const table = new (this.dh as any).TreeTable({
columns,
groupedColumns,
size,
sort,
});
table.copy = jest.fn(() => Promise.resolve(table));
return table;
}
Expand Down
29 changes: 29 additions & 0 deletions packages/iris-grid/src/IrisGridTreeTableModel.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import dh from '@deephaven/jsapi-shim';
import IrisGridTestUtils from './IrisGridTestUtils';
import IrisGridTreeTableModel from './IrisGridTreeTableModel';

const irisGridTestUtils = new IrisGridTestUtils(dh);

describe('IrisGridTreeTableModel', () => {
const expectedVirtualColumn = expect.objectContaining({
name: '__DH_UI_GROUP__',
displayName: 'Group',
isProxy: true,
});
const columns = irisGridTestUtils.makeColumns();

test.each([
[0, columns, columns],
[1, columns, columns],
[2, columns, [expectedVirtualColumn, ...columns]],
[columns.length, columns, [expectedVirtualColumn, ...columns]],
])(
`create virtual columns with group length %`,
(groupedLength, allColumns, expected) => {
const groupedColumns = allColumns.slice(0, groupedLength);
const table = irisGridTestUtils.makeTreeTable(allColumns, groupedColumns);
const model = new IrisGridTreeTableModel(dh, table);
expect(model.columns).toEqual(expected);
}
);
});
51 changes: 27 additions & 24 deletions packages/iris-grid/src/IrisGridTreeTableModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,30 +41,33 @@ class IrisGridTreeTableModel extends IrisGridTableModelTemplate<
inputTable: InputTable | null = null
) {
super(dh, table, formatter, inputTable);
this.virtualColumns = [
{
name: '__DH_UI_GROUP__',
displayName: 'Group',
type: TableUtils.dataType.STRING,
constituentType: TableUtils.dataType.STRING,
isPartitionColumn: false,
isSortable: false,
isProxy: true,
description: 'Key column',
filter: () => {
throw new Error('Filter not implemented for virtual column');
},
sort: () => {
throw new Error('Sort not implemented virtual column');
},
formatColor: () => {
throw new Error('Color not implemented for virtual column');
},
formatRowColor: () => {
throw new Error('Color not implemented for virtual column');
},
},
];
this.virtualColumns =
table.groupedColumns.length > 1
? [
{
name: '__DH_UI_GROUP__',
displayName: 'Group',
type: TableUtils.dataType.STRING,
constituentType: TableUtils.dataType.STRING,
isPartitionColumn: false,
isSortable: false,
isProxy: true,
description: 'Key column',
filter: () => {
throw new Error('Filter not implemented for virtual column');
},
sort: () => {
throw new Error('Sort not implemented virtual column');
},
formatColor: () => {
throw new Error('Color not implemented for virtual column');
},
formatRowColor: () => {
throw new Error('Color not implemented for virtual column');
},
},
]
: [];
}

applyBufferedViewport(
Expand Down

0 comments on commit e8196bb

Please sign in to comment.