Skip to content

Commit 3a83d3f

Browse files
authored
fix: DH-22174: ui.table works with rollup and tree tables (#1343)
- Added e2e tests to verify ui.table works with rollup and tree tables - Can set formatting on the table as well - CAVEAT: Does not support `_if` conditional formatting when used with rollup tables. This will error early when validating the Python. - Verified the code snippet from the ticket works correctly: ``` from deephaven import new_table, agg from deephaven.column import string_col, double_col from deephaven import ui raw = new_table([ string_col("Group", ["A", "A", "A", "B", "B", "B"]), string_col("Person", ["Alice", "Alice", "Bob", "Bob", "Carol", "Carol"]), double_col("Value1", [1234.56, -789.12, 3210.99, 4567.89, -123.45, 9876.54]), double_col("Value2", [50000.33, 30000.77, 45000.50, 120000.11, 80000.99, 200000.44]), double_col("Pct", [0.0512, 0.0623, 0.0310, 0.0234, 0.0345, 0.0178]), ]) rollup_table = raw.rollup( aggs=[agg.sum_(["Value1", "Value2"]), agg.avg(["Pct"])], by=["Group", "Person"], include_constituents=True, ) rollup_fmt = ui.table( rollup_table, format_=[ ui.TableFormat(background_color="fg"), ui.TableFormat(cols="Group", color="accent"), ], ) ```
1 parent 6fa17ba commit 3a83d3f

12 files changed

Lines changed: 408 additions & 234 deletions

File tree

plugins/ui/src/deephaven/ui/components/table.py

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
from dataclasses import dataclass, field
33
from typing import Literal, Any, Optional
44
import logging
5-
from deephaven.table import Table
5+
from deephaven.table import RollupTable, TreeTable
66
from ..elements import Element, resolve
77
from ..elements.UriElement import UriElement
88
from .types import AlignSelf, DimensionValue, JustifySelf, LayoutFlex, Position
@@ -16,6 +16,7 @@
1616
RowPressCallback,
1717
ResolvableContextMenuItem,
1818
SelectionChangeCallback,
19+
TableLike,
1920
)
2021
from .._internal import dict_to_react_props, RenderContext
2122

@@ -157,22 +158,34 @@ class TableDatabar:
157158
markers: list[dict[str, Any]] | None = None
158159

159160

160-
def _validate_table_format(format_: list[TableFormat] | TableFormat) -> None:
161+
def _validate_table_format(
162+
format_: list[TableFormat] | TableFormat,
163+
table: TableLike | UriElement | str,
164+
) -> None:
161165
"""Validate format rules for the table.
162166
163167
Args:
164168
format_: A formatting rule or list of formatting rules to validate.
169+
table: The table the format rules will be applied to. Used to validate
170+
that rules are compatible with the table type.
165171
166172
Raises:
167173
ValueError: If a format rule has a mode but no cols.
168174
ValueError: If a format rule has a heatmap but no cols.
169175
ValueError: If a heatmap gradient has fewer than 2 colors.
176+
ValueError: If a format rule uses if_ on a rollup or tree table.
170177
"""
171178
format_list = format_ if isinstance(format_, list) else [format_]
179+
is_hierarchical = isinstance(table, (RollupTable, TreeTable))
172180
for f in format_list:
173181
if f.mode is not None and f.cols is None:
174182
raise ValueError("TableFormat with mode requires cols to be specified.")
175183

184+
if is_hierarchical and f.if_ is not None:
185+
raise ValueError(
186+
"TableFormat if_ is not supported on rollup or tree tables."
187+
)
188+
176189
if isinstance(f.color, TableHeatmap):
177190
if f.cols is None:
178191
raise ValueError(
@@ -292,7 +305,7 @@ class table(Element):
292305

293306
def __init__(
294307
self,
295-
table: Table | UriElement | str,
308+
table: TableLike | UriElement | str,
296309
*,
297310
format_: TableFormat | list[TableFormat] | None = None,
298311
on_row_press: RowPressCallback | None = None,
@@ -369,7 +382,7 @@ def __init__(
369382
)
370383

371384
if format_ is not None:
372-
_validate_table_format(format_)
385+
_validate_table_format(format_, table)
373386

374387
props["table"] = resolve(table) if isinstance(table, str) else table
375388
del props["self"]

plugins/ui/src/deephaven/ui/types/types.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222

2323
from deephaven import SortDirection
2424
from deephaven.dtypes import DType
25-
25+
from deephaven.table import Table, RollupTable, TreeTable
2626

2727
# Color values for the DH color palette exposed to end users in spectrum components
2828
# https://github.com/deephaven/web-client-ui/blob/main/packages/components/src/theme/colorUtils.ts
@@ -458,6 +458,7 @@ class SliderChange(TypedDict):
458458
ContextMenuMode = Union[ContextMenuModeOption, List[ContextMenuModeOption], None]
459459
# TODO: Fill in the list of Deephaven Colors we allow
460460
LockType = Literal["shared", "exclusive"]
461+
TableLike = Union[Table, RollupTable, TreeTable]
461462
QuickFilterExpression = str
462463
RowData = Dict[ColumnName, Any]
463464
ColumnData = List[Any]
Lines changed: 256 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,256 @@
1+
import type { dh } from '@deephaven/jsapi-types';
2+
import {
3+
DATABAR_MIN_SUFFIX,
4+
DATABAR_MAX_SUFFIX,
5+
HEATMAP_MIN_SUFFIX,
6+
HEATMAP_MAX_SUFFIX,
7+
} from './UITableUtils';
8+
9+
export interface UITableLayoutHints {
10+
frontColumns?: string[];
11+
frozenColumns?: string[];
12+
backColumns?: string[];
13+
hiddenColumns?: string[];
14+
columnGroups?: dh.ColumnGroup[];
15+
}
16+
17+
/**
18+
* Subset of the dh.Table / dh.TreeTable surface area used by the proxy base
19+
* class. Both Table and TreeTable expose these members, so the base proxy can
20+
* operate on either.
21+
*/
22+
export interface ProxyableTable {
23+
readonly columns: dh.Column[];
24+
readonly customColumns: dh.CustomColumn[];
25+
readonly isClosed: boolean;
26+
applyCustomColumns: (
27+
customColumns: Array<string | dh.CustomColumn>
28+
) => Array<dh.CustomColumn>;
29+
close: () => void;
30+
setViewport: (
31+
firstRow: number,
32+
lastRow: number,
33+
columns?: Array<dh.Column> | undefined | null,
34+
updateIntervalMs?: number | undefined | null
35+
) => dh.TableViewportSubscription | void;
36+
}
37+
38+
/**
39+
* Base class for proxying a JS API Table or TreeTable.
40+
*
41+
* The underlying table passed to the constructor may be modified, so callers
42+
* should pass a copy. Methods implemented on this class (or a subclass) take
43+
* precedence over the underlying table's methods; everything else is proxied
44+
* through to the table.
45+
*/
46+
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
47+
// @ts-ignore
48+
class JsBaseTableProxy<T extends ProxyableTable, R> {
49+
static HIDDEN_COLUMN_SUFFIXES = [
50+
DATABAR_MIN_SUFFIX,
51+
DATABAR_MAX_SUFFIX,
52+
HEATMAP_MIN_SUFFIX,
53+
HEATMAP_MAX_SUFFIX,
54+
'__FORMAT',
55+
];
56+
57+
protected table: T;
58+
59+
/**
60+
* Keep a stable reference to all, visible, and hidden columns.
61+
* Only update when needed.
62+
*/
63+
private stableColumns: {
64+
allColumns: dh.Column[];
65+
visibleColumns: dh.Column[];
66+
hiddenColumns: dh.Column[];
67+
};
68+
69+
protected originalCustomColumns: dh.CustomColumn[];
70+
71+
private onClose: () => void;
72+
73+
layoutHints: dh.LayoutHints | null = null;
74+
75+
constructor({
76+
table,
77+
layoutHints,
78+
onClose,
79+
}: {
80+
table: T;
81+
layoutHints: UITableLayoutHints;
82+
onClose: () => void;
83+
}) {
84+
this.table = table;
85+
this.originalCustomColumns = table.customColumns;
86+
this.onClose = onClose;
87+
88+
this.stableColumns = {
89+
allColumns: [],
90+
visibleColumns: [],
91+
hiddenColumns: [],
92+
};
93+
94+
this.layoutHints = {
95+
frontColumns: null,
96+
frozenColumns: null,
97+
backColumns: null,
98+
hiddenColumns: null,
99+
columnGroups: null,
100+
...layoutHints,
101+
areSavedLayoutsAllowed: false,
102+
};
103+
104+
// eslint-disable-next-line no-constructor-return
105+
return new Proxy(this, {
106+
// We want to use any properties on the proxy model if defined
107+
// If not, then proxy to the underlying model
108+
get(target, prop, receiver) {
109+
// Walk the prototype chain so getters defined on subclasses or this
110+
// base class are found.
111+
let proto = Object.getPrototypeOf(target);
112+
while (proto != null && proto !== Object.prototype) {
113+
const descriptor = Object.getOwnPropertyDescriptor(proto, prop);
114+
if (descriptor?.get != null) {
115+
return Reflect.get(target, prop, receiver);
116+
}
117+
if (descriptor != null && typeof descriptor.value === 'function') {
118+
return descriptor.value.bind(target);
119+
}
120+
proto = Object.getPrototypeOf(proto);
121+
}
122+
123+
// Does this instance implement the property
124+
if (Object.prototype.hasOwnProperty.call(target, prop)) {
125+
return Reflect.get(target, prop, receiver);
126+
}
127+
128+
const value = Reflect.get(target.table, prop, receiver);
129+
if (typeof value === 'function') {
130+
return value.bind(target.table);
131+
}
132+
return value;
133+
},
134+
set(target, prop, value) {
135+
// Walk the prototype chain looking for a setter.
136+
let proto = Object.getPrototypeOf(target);
137+
while (proto != null && proto !== Object.prototype) {
138+
const descriptor = Object.getOwnPropertyDescriptor(proto, prop);
139+
if (descriptor?.set != null) {
140+
return Reflect.set(target, prop, value);
141+
}
142+
proto = Object.getPrototypeOf(proto);
143+
}
144+
145+
if (Object.prototype.hasOwnProperty.call(target, prop)) {
146+
return Reflect.set(target, prop, value);
147+
}
148+
149+
return Reflect.set(target.table, prop, value, target.table);
150+
},
151+
});
152+
}
153+
154+
/**
155+
* Update the stable columns object if needed.
156+
* This lets us keep a stable array for columns unless the underlying table changes.
157+
*/
158+
private updateDisplayedColumns(): void {
159+
if (this.stableColumns.allColumns !== this.table.columns) {
160+
this.stableColumns.allColumns = this.table.columns;
161+
162+
this.stableColumns.visibleColumns = this.table.columns.filter(
163+
column =>
164+
!JsBaseTableProxy.HIDDEN_COLUMN_SUFFIXES.some(suffix =>
165+
column.name.endsWith(suffix)
166+
)
167+
);
168+
169+
this.stableColumns.hiddenColumns = this.table.columns.filter(column =>
170+
JsBaseTableProxy.HIDDEN_COLUMN_SUFFIXES.some(suffix =>
171+
column.name.endsWith(suffix)
172+
)
173+
);
174+
}
175+
}
176+
177+
close(): void {
178+
// Something causes close to get called twice which will throw some log spam if we try to close the table again
179+
if (!this.table.isClosed) {
180+
this.onClose();
181+
this.table.close();
182+
}
183+
}
184+
185+
applyCustomColumns(
186+
customColumns: Array<string | dh.CustomColumn>
187+
): Array<dh.CustomColumn> {
188+
return this.table.applyCustomColumns([
189+
...this.originalCustomColumns,
190+
...customColumns,
191+
]);
192+
}
193+
194+
get columns(): dh.Column[] {
195+
this.updateDisplayedColumns();
196+
return this.stableColumns.visibleColumns;
197+
}
198+
199+
get hiddenColumns(): dh.Column[] {
200+
this.updateDisplayedColumns();
201+
return this.stableColumns.hiddenColumns;
202+
}
203+
204+
setViewport(
205+
firstRow: number,
206+
lastRow: number,
207+
columns?: Array<dh.Column> | undefined | null,
208+
updateIntervalMs?: number | undefined | null
209+
): R {
210+
if (columns == null) {
211+
return this.table.setViewport(
212+
firstRow,
213+
lastRow,
214+
columns,
215+
updateIntervalMs
216+
) as unknown as R;
217+
}
218+
219+
const allColumns = columns.concat(this.hiddenColumns);
220+
const viewportSubscription = this.table.setViewport(
221+
firstRow,
222+
lastRow,
223+
allColumns,
224+
updateIntervalMs
225+
);
226+
// TreeTable.setViewport returns void; avoid wrapping undefined in a Proxy
227+
if (viewportSubscription == null) {
228+
return viewportSubscription as unknown as R;
229+
}
230+
return new Proxy(viewportSubscription, {
231+
get: (target, prop, receiver) => {
232+
// Need to proxy setViewport on the subscription in case it is changed
233+
// without creating an entirely new subscription
234+
if (prop === 'setViewport') {
235+
return (
236+
first: number,
237+
last: number,
238+
cols?: dh.Column[] | null,
239+
interval?: number | null
240+
) => {
241+
if (cols == null) {
242+
return target.setViewport(first, last, cols, interval);
243+
}
244+
245+
const proxyAllColumns = cols.concat(this.hiddenColumns);
246+
247+
return target.setViewport(first, last, proxyAllColumns, interval);
248+
};
249+
}
250+
return Reflect.get(target, prop, receiver);
251+
},
252+
}) as unknown as R;
253+
}
254+
}
255+
256+
export default JsBaseTableProxy;

0 commit comments

Comments
 (0)