-
Notifications
You must be signed in to change notification settings - Fork 15
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: add use_table_listener hook #115
Changes from all commits
226b08a
e2b5bda
e2ef0c3
b48d8ed
ed0c232
1d7ea31
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
jnumainville marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,104 @@ | ||
from __future__ import annotations | ||
|
||
from functools import partial | ||
from typing import Callable, Literal | ||
|
||
from deephaven.table import Table | ||
from deephaven.table_listener import listen, TableUpdate, TableListener | ||
from deephaven.execution_context import get_exec_ctx, ExecutionContext | ||
|
||
from .use_effect import use_effect | ||
|
||
LockType: Literal["shared", "exclusive"] | ||
jnumainville marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
|
||
def listener_with_ctx( | ||
exec_ctx: ExecutionContext, | ||
listener: Callable[[TableUpdate, bool], None], | ||
update: TableUpdate, | ||
is_replay: bool, | ||
) -> None: | ||
""" | ||
Call the listener within an execution context. | ||
|
||
Args: | ||
exec_ctx: ExecutionContext: The execution context to use. | ||
listener: Callable[[TableUpdate, bool], None]: The listener to call. | ||
update: TableUpdate: The update to pass to the listener. | ||
is_replay: bool: Whether the update is a replay. | ||
""" | ||
with exec_ctx: | ||
listener(update, is_replay) | ||
|
||
|
||
def with_ctx( | ||
listener: Callable[[TableUpdate, bool], None] | ||
) -> partial[[TableUpdate, bool], None]: | ||
""" | ||
Wrap the listener in an execution context. | ||
|
||
Args: | ||
listener: The listener to wrap. | ||
|
||
Returns: | ||
partial[[TableUpdate, bool], None]: The wrapped listener. | ||
""" | ||
return partial(listener_with_ctx, get_exec_ctx(), listener) | ||
|
||
|
||
def wrap_listener( | ||
listener: Callable[[TableUpdate, bool], None] | TableListener | ||
) -> partial[[TableUpdate, bool], None]: | ||
""" | ||
Wrap the listener in an execution context. | ||
|
||
Args: | ||
listener: Callable[[TableUpdate, bool], None]: The listener to wrap. | ||
|
||
Returns: | ||
partial[[TableUpdate, bool], None]: The wrapped listener. | ||
""" | ||
if isinstance(listener, TableListener): | ||
return with_ctx(listener.on_update) | ||
elif callable(listener): | ||
return with_ctx(listener) | ||
|
||
|
||
def use_table_listener( | ||
table: Table, | ||
listener: Callable[[TableUpdate, bool], None] | TableListener, | ||
description: str | None = None, | ||
do_replay: bool = False, | ||
replay_lock: LockType = "shared", | ||
) -> None: | ||
""" | ||
Listen to a table and call a listener when the table updates. | ||
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 think this needs more red flashing lights and warning tape - by using this on a single table in a component, the rest of the component is restricted from snapshotting (i.e. calling to_pandas, among other things) any table that isn't this table or one of its ancestors. The Could we have a safeguard of some kind, ensure that a component either has a listener or has a snapshot, but never both? Or, could we have an explicit "call set on this use_state, but only render in another thread (after the current update cycle is finished)" - state changes could accumulate, and run "later" to actually re-render and notify the client? There is still risk with this - an update could be missed (but a snapshot of table state will always be consistent), though as I understand it this is consistent with the react model? (This probably invites another discussion of "what if I have two different to_pandas() calls, how do I know to use the appropriate lock for the entire component", but we could handle that separately as this evolves?) 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. @niloc132 All these caveats you mention - we could really use more documentation on using table listeners, if that's the case: https://deephaven.io/core/docs/how-to-guides/table-listeners-python/ Anyways - for I think that would address the main part of "red flashing lights" part you're talking about? I think the first usage of this |
||
|
||
Args: | ||
table: Table: The table to listen to. | ||
listener: Callable[[TableUpdate, bool], None] | TableListener: Either a function or a TableListener with an | ||
on_update function. The function must take a TableUpdate and is_replay bool. | ||
description: str | None: An optional description for the UpdatePerformanceTracker to append to the listener’s | ||
entry description, default is None. | ||
do_replay: bool: Whether to replay the initial snapshot of the table, default is False. | ||
replay_lock: LockType: The lock type used during replay, default is ‘shared’, can also be ‘exclusive’. | ||
""" | ||
|
||
def start_listener() -> Callable[[], None]: | ||
""" | ||
Start the listener. Returns a function that can be called to stop the listener by the use_effect hook. | ||
|
||
Returns: | ||
Callable[[], None]: A function that can be called to stop the listener by the use_effect hook. | ||
""" | ||
handle = listen( | ||
table, | ||
jnumainville marked this conversation as resolved.
Show resolved
Hide resolved
|
||
wrap_listener(listener), | ||
description=description, | ||
do_replay=do_replay, | ||
replay_lock=replay_lock, | ||
) | ||
|
||
return lambda: handle.stop() | ||
|
||
use_effect(start_listener, [table, listener, description, do_replay, replay_lock]) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,7 @@ | ||
import threading | ||
import unittest | ||
from operator import itemgetter | ||
from time import sleep | ||
from typing import Callable | ||
from unittest.mock import Mock | ||
from .BaseTest import BaseTestCase | ||
|
@@ -133,6 +135,33 @@ def _test_memo(fn=lambda: "foo", a=1, b=2): | |
self.assertEqual(result, "biz") | ||
self.assertEqual(mock.call_count, 1) | ||
|
||
def test_table_listener(self): | ||
from deephaven.ui.hooks import use_table_listener | ||
from deephaven import time_table, new_table, DynamicTableWriter | ||
from deephaven.table_listener import TableUpdate | ||
import deephaven.dtypes as dht | ||
|
||
column_definitions = {"Numbers": dht.int32, "Words": dht.string} | ||
|
||
table_writer = DynamicTableWriter(column_definitions) | ||
table = table_writer.table | ||
|
||
event = threading.Event() | ||
|
||
def listener(update: TableUpdate, is_replay: bool) -> None: | ||
nonlocal event | ||
event.set() | ||
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. Nice, yea I like this way better. |
||
|
||
def _test_table_listener(replayed_table_val=table, listener_val=listener): | ||
use_table_listener(replayed_table_val, listener_val) | ||
|
||
render_hook(_test_table_listener) | ||
|
||
table_writer.write_row(1, "Testing") | ||
|
||
if not event.wait(timeout=1.0): | ||
assert False, "listener was not called" | ||
|
||
|
||
if __name__ == "__main__": | ||
unittest.main() |
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 look smoother using a TablePublisher, as we wouldn't have to recreate the table each time. Would've also been fine to just output text, as creating another table isn't really the focus of this example. This is fine though.