-
Notifications
You must be signed in to change notification settings - Fork 81
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.
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
feat: JS Clients can read constituent data in unaggregated columns #6663
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No test plan or example for how to test this with JS?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A minor nitpick for cleanup.
Tested with a few different tables:
from deephaven import agg, empty_table, ui
from deephaven.plot import express as dx
stocks = dx.data.stocks()
r = stocks.rollup(aggs=agg.sum_(["Price"]), by=["Sym", "Exchange"], include_constituents=True)
r2 = stocks.rollup(aggs=agg.sum_(["Price"]), by=["Sym", "Exchange"], include_constituents=False)
r3 = stocks.view(["Sym", "Exchange", "Size"]).rollup(aggs=agg.sum_(["Price"]), by=["Sym", "Exchange"], include_constituents=True)
constituentColumns.put(column.getName(), column); | ||
} else { | ||
columns[columns.length] = column; | ||
if (definition.isRollupGroupByColumn() && !definition.isRollupConstituentNodeColumn()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need for the && !definition.isRollupConstituentNodeColumn()
, as that's always true in this else
block
if (definition.isRollupGroupByColumn() && !definition.isRollupConstituentNodeColumn()) { | |
if (definition.isRollupGroupByColumn()) { |
Constituent columns are available for snapshot by the server, but the JS API until now has not made them available to users. This patch exposes those columns as part of the
dh.TreeTable
object, and allows them to be subscribed to.Fixes DH-18431
Fixes #3303