-
Notifications
You must be signed in to change notification settings - Fork 16
docs: deephaven express ui filtering #1157
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
# Deephaven Express UI Filtering | ||
|
||
## Options | ||
|
||
1. Wrapper around table, similar to current oneClick API, e.g. `p = dx.line(one_click(t), x="x", y="y")` | ||
|
||
2. Parameter on the plot: `dx.line(t, x="Timestamp", y="Last", input_filters=["Sym"])` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This would require baking in a new parameter into each deephaven-express API, and does not address any custom widgets. |
||
|
||
3. Wrapper around the plot, e.g. `p = ui.filterable(table, lambda t: dx.line(...))` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With this, we could automatically display a shade for the widget until we actually have an input, and only then create the plot. This would also work with custom widgets, and also work within I kind of like this. It seems to address all the cases. What are the downsides? I guess that it creates another scope/lambda that needs to be run, rather than being used like a typical state variable within React? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like option 3 the most, it could also be backed by something like option 4 for maximum flexibility. The only (extremely nitpicky) downsides I can think of are
I also want to note that the upside is huge though in terms of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I also like this option the best. I like the idea of something generic that can be used to filter more than just plots. It would force the use of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. On the topic of something more generic, are you guys planning to require I know this PR is about DX, but mentioning this here in case the designs are coupled. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tables should always react to input filters on the dashboard. Matt will be working on that: https://deephaven.atlassian.net/issues/DH-18354 |
||
|
||
4. State object `[state] = ui.dashboard_inputs()` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This option may be nice for flexibility with custom widgets and
from deephaven import read_csv
from deephaven.plot.selectable_dataset import one_click
from deephaven.plot.figure import Figure
source = read_csv(
"https://media.githubusercontent.com/media/deephaven/examples/main/CryptoCurrencyHistory/CSV/CryptoTrades_20210922.csv"
)
# require_all_filters means we need to filter the table first
oc = one_click(t=source, by=["Instrument"], require_all_filters=True)
plot = Figure().plot_xy(series_name="Plot", t=oc, x="Timestamp", y="Price").show() There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thinking about this further, you could easily build the |
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.
While the suggestion is doing it just similar to the existing API... I imagine it would be more like:
Basically the same as the existing
one_click
API. This would require changes todx
I think - @jnumainville what is DX currently all looking for on the server with thetable
passed in? Column schema, and in the case of partitioned table listening to the key table for updates?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.
It depends. The table that is passed in may be transformed completely (like histogram) have some columns added (like treemap). So many times the original table isn't held on to.
Then yeah, the column schema is pulled to try to help plotly process them, and the partitioned table is listened to for updates. Column names are also pulled at various times to ensure there is no collisions.
Uh oh!
There was an error while loading. Please reload this page.
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.
This would require about as much work as option 2, because the bulk of the work here here is the need for regeneration after a client -> server message. This is slightly better I suppose because in theory the result of
one_click(t)
would contain a table thatdx
could pull out. And I suppose we could also put a listener intoone_click(t)
as well, but either way it gets ugly and requires plenty ofdx
work.