-
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?
Conversation
|
||
3. Wrapper around the plot, e.g. `p = ui.filterable(table, lambda t: dx.line(...))` | ||
|
||
4. State object `[state] = ui.dashboard_inputs()` |
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 option may be nice for flexibility with custom widgets and @ui.component
s anyway, but I do have some questions about it:
- First, I think it should just be
state = ui.dashboard_inputs()
. I don't think we need aset_state
... - What is the data structure used for
state
? I'm guessing similar to the IrisGridPanelProps, e.g.{ name: string, type: string, value: string }[]
- This doesn't address how we show on a chart panel that the input needs to be filtered. Assuming we'd still be passing a table (which would be derived via filtering another table with the provided
state
) intodx.line
or whichever plotly-express function. Right now one of the benefits ofone_click
is it displays on the chart the need for a filter, and clicking that button creates an input filter for you, e.g.
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 comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking about this further, you could easily build the ui.filterable
component on top of this hook.
To implement this hook, we'd need a some of use_client_context
hook, which would subscribe to a specific context ID and the client would provide the valid. In this case, inputFilters
; but we could add other contexts in the future.
|
||
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 comment
The 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.
|
||
## Options | ||
|
||
1. Wrapper around table, similar to current oneClick API, e.g. `p = dx.line(one_click(t), x="x", y="y")` |
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:
p = dx.line(ui.inputs(t, require_all_filters=True), x="x", y="y")
Basically the same as the existing one_click
API. This would require changes to dx
I think - @jnumainville what is DX currently all looking for on the server with the table
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.
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 that dx
could pull out. And I suppose we could also put a listener into one_click(t)
as well, but either way it gets ugly and requires plenty of dx
work.
|
||
2. Parameter on the plot: `dx.line(t, x="Timestamp", y="Last", input_filters=["Sym"])` | ||
|
||
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 comment
The 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 @ui.component
s. I think only downside is you wouldn't be able to use the input filter value server-side so much as it would be specifically taking in a Table
as input (perhaps some other parameters, such as require_all_filters
), and then the filter would be applied on that table resulting in the output table being used as a parameter for a lambda
. I guess that makes sense though, as the table provided as input could also be an input for the type of filter (e.g. Dropdown filter can have a dropdown of the values for the different column types)... and the lambda t
could easily take another parameter that is the raw input values as well ... or we could have a different object, such as ui.inputs(lambda input_filters: dx.line(...))
if we wanted to be flexible with it and not pass the tables in as data sources to the input filters.
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 comment
The 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
- The lambda as Bender mentioned. We could make this slightly simpler by adding a function to
dx.Deephaven.Figure
likefilter(data): return self.give_me_a_new_figure
(basically a chart factory method). I'm not sure if that's worth doing since it wouldn't matter for custom widgets unless we put this out there as an option instead of lambdas. - It requires
dh.ui
, but it sounds like we are going that direction with all but option 2 anyways - In the case of
dx
the figure is completely regenerated, but that isn't as major as it sounds because that's basically what happens anyways. There are a couple things to help with this too, but they are implementation details.
I also want to note that the upside is huge though in terms of dx
in comparison to options 2 and 3 as this would require little to no work and maintenance, dependent on implementation details.
Additionally, if we're making this generic for all sorts of widgets this is fantastic for consistency rather than some special solution for dx
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.
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 dh.ui
, but our current solution requires the use of one_click
, so I think it's a good trade.
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.
On the topic of something more generic, are you guys planning to require ui.filterable
for even tables within dh.ui components? Unlike plotting, that extra step would be a departure from the previous flow/expectation, which is to require code to get filterable plots, but get filterable tables for free.
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 comment
The 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
Discussion on how best to implement UI filtering of dx plots (e.g. input filter and linker). Tickets:
https://deephaven.atlassian.net/browse/DH-18281
https://deephaven.atlassian.net/browse/DH-18840