Skip to content

Commit

Permalink
fix: Tighten use_memo dependency types (#356)
Browse files Browse the repository at this point in the history
Fixes #355 
Only allow tuple and list, and throw if neither.
  • Loading branch information
jnumainville authored Mar 13, 2024
1 parent 7e7302e commit 48dea18
Show file tree
Hide file tree
Showing 10 changed files with 36 additions and 20 deletions.
10 changes: 5 additions & 5 deletions docker/data/storage/notebooks/DEMO.md
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ def order_table():
),
[],
)
t = ui.use_memo(lambda: blink_to_append_only(blink_table), {blink_table})
t = ui.use_memo(lambda: blink_to_append_only(blink_table), [blink_table])

def submit_order(order_sym, order_size, side):
publisher.add(
Expand Down Expand Up @@ -437,18 +437,18 @@ def multiwave():
f"y_tan={amplitude}*Math.tan({frequency}*x+{phase})",
]
),
{amplitude, frequency, phase},
[amplitude, frequency, phase],
)
p_sin = use_memo(
lambda: Figure().plot_xy(series_name="Sine", t=t, x="x", y="y_sin").show(), {t}
lambda: Figure().plot_xy(series_name="Sine", t=t, x="x", y="y_sin").show(), [t]
)
p_cos = use_memo(
lambda: Figure().plot_xy(series_name="Cosine", t=t, x="x", y="y_cos").show(),
{t},
[t],
)
p_tan = use_memo(
lambda: Figure().plot_xy(series_name="Tangent", t=t, x="x", y="y_tan").show(),
{t},
[t],
)

return ui.column(
Expand Down
3 changes: 2 additions & 1 deletion plugins/ui/src/deephaven/ui/hooks/use_callback.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@
from typing import Callable, Any, Sequence

from .use_ref import use_ref, Ref
from ..types import Dependencies


def use_callback(func: Callable, dependencies: set[Any] | Sequence[Any]) -> Callable:
def use_callback(func: Callable, dependencies: Dependencies) -> Callable:
"""
Create a stable handle for a callback function. The callback will only be recreated if the dependencies change.
Expand Down
3 changes: 2 additions & 1 deletion plugins/ui/src/deephaven/ui/hooks/use_effect.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@
from .use_ref import use_ref, Ref
from deephaven.liveness_scope import LivenessScope
from .._internal import get_context
from ..types import Dependencies


def use_effect(func: Callable[[], Any], dependencies: set[Any] | Sequence[Any]):
def use_effect(func: Callable[[], Any], dependencies: Dependencies):
"""
Call a function when the dependencies change. Optionally return a cleanup function to be called when dependencies change again or component is unmounted.
Expand Down
4 changes: 2 additions & 2 deletions plugins/ui/src/deephaven/ui/hooks/use_execution_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,6 @@ def use_execution_context(
Returns:
A callable that will take any callable and invoke it within the current exec context
"""
exec_ctx = use_memo(lambda: exec_ctx if exec_ctx else get_exec_ctx(), {exec_ctx})
exec_fn = use_memo(lambda: partial(func_with_ctx, exec_ctx), {exec_ctx})
exec_ctx = use_memo(lambda: exec_ctx if exec_ctx else get_exec_ctx(), [exec_ctx])
exec_fn = use_memo(lambda: partial(func_with_ctx, exec_ctx), [exec_ctx])
return exec_fn
3 changes: 2 additions & 1 deletion plugins/ui/src/deephaven/ui/hooks/use_liveness_scope.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,10 @@
from .use_ref import use_ref, Ref
from typing import Callable
from deephaven.liveness_scope import LivenessScope
from ..types import Dependencies


def use_liveness_scope(func: Callable, dependencies: set) -> Callable:
def use_liveness_scope(func: Callable, dependencies: Dependencies) -> Callable:
"""
Wraps a Callable in a liveness scope, and ensures that when invoked, if that callable
causes the component to render, the scope will be retained by that render pass. It is
Expand Down
8 changes: 7 additions & 1 deletion plugins/ui/src/deephaven/ui/hooks/use_memo.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@
from .._internal import ValueWithLiveness, get_context
from typing import Any, Callable, TypeVar, cast, Union, Sequence
from deephaven.liveness_scope import LivenessScope
from ..types import Dependencies

T = TypeVar("T")


def use_memo(func: Callable[[], T], dependencies: set[Any] | Sequence[Any]) -> T:
def use_memo(func: Callable[[], T], dependencies: Dependencies) -> T:
"""
Memoize the result of a function call. The function will only be called again if the dependencies change.
Expand All @@ -19,6 +20,11 @@ def use_memo(func: Callable[[], T], dependencies: set[Any] | Sequence[Any]) -> T
Returns:
The memoized result of the function call.
"""
if not isinstance(dependencies, (list, tuple)):
raise TypeError(
f"dependencies must be a list or tuple, got {type(dependencies)}"
)

deps_ref: Ref[set[Any] | Sequence[Any] | None] = use_ref(None)
value_ref: Ref[ValueWithLiveness[T | None]] = use_ref(
ValueWithLiveness(value=cast(Union[T, None], None), liveness_scope=None)
Expand Down
2 changes: 1 addition & 1 deletion plugins/ui/src/deephaven/ui/hooks/use_table_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ def use_table_data(
table_updated = lambda: _set_new_data(table, sentinel, set_data, set_is_sentinel)

ui.use_table_listener(
table, partial(_on_update, ctx, table_updated, executor_name), set()
table, partial(_on_update, ctx, table_updated, executor_name), []
)

return transform(data, is_sentinel)
6 changes: 3 additions & 3 deletions plugins/ui/src/deephaven/ui/hooks/use_table_listener.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from deephaven.execution_context import get_exec_ctx, ExecutionContext

from .use_effect import use_effect
from ..types import LockType
from ..types import LockType, Dependencies


def listener_with_ctx(
Expand Down Expand Up @@ -67,7 +67,7 @@ def wrap_listener(
def use_table_listener(
table: Table,
listener: Callable[[TableUpdate, bool], None] | TableListener,
dependencies: set[Any] | Sequence[Any],
dependencies: Dependencies,
description: str | None = None,
do_replay: bool = False,
replay_lock: LockType = "shared",
Expand Down Expand Up @@ -109,5 +109,5 @@ def start_listener() -> Callable[[], None]:

use_effect(
start_listener,
{table, listener, description, do_replay, replay_lock} | set(dependencies),
[table, listener, description, do_replay, replay_lock] + list(dependencies),
)
1 change: 1 addition & 0 deletions plugins/ui/src/deephaven/ui/types/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,3 +105,4 @@ class RowDataValue(CellData):
# Stringable is a type that is naturally convertible to a string
Stringable = Union[str, int, float, bool]
Key = Stringable
Dependencies = Union[Tuple[Any], List[Any]]
16 changes: 11 additions & 5 deletions plugins/ui/test/deephaven/ui/test_hooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ def test_memo(self):
from deephaven.ui.hooks import use_memo

def _test_memo(fn=lambda: "foo", a=1, b=2):
return use_memo(fn, {a, b})
return use_memo(fn, [a, b])

# Initial render
render_result = render_hook(_test_memo)
Expand Down Expand Up @@ -140,6 +140,12 @@ def _test_memo(fn=lambda: "foo", a=1, b=2):
self.assertEqual(result, "biz")
self.assertEqual(mock.call_count, 1)

def _test_memo_set(fn=lambda: "foo"):
return use_memo(fn, {})

# passing in a non-list/tuple for dependencies should raise a TypeError
self.assertRaises(TypeError, render_hook, _test_memo_set)

def verify_table_updated(self, table_writer, table, update):
from deephaven.ui.hooks import use_table_listener
from deephaven.table_listener import TableUpdate
Expand Down Expand Up @@ -541,7 +547,7 @@ def start_thread():
thread.start()
thread.join()

use_memo(start_thread, set())
use_memo(start_thread, [])

render_hook(_test_execution_context)

Expand All @@ -564,7 +570,7 @@ def start_thread():
thread = threading.Thread(target=thread_func)
thread.start()

use_memo(start_thread, set())
use_memo(start_thread, [])

render_hook(_test_execution_context)

Expand Down Expand Up @@ -595,7 +601,7 @@ def _test_reused_tables():
a, set_a = use_state(lambda: table.where("X=1"))

# When "a" changes, recompute table - don't return or otherwise track this table w.r.t. liveness
replace_a = use_liveness_scope(lambda: set_a(table.where("X=2")), set())
replace_a = use_liveness_scope(lambda: set_a(table.where("X=2")), [])

return a.size

Expand Down Expand Up @@ -664,7 +670,7 @@ def helper():
now = dh_now()
return table.where("Timestamp > now").last_by(by=["X"])

local_rows = use_memo(helper, {a})
local_rows = use_memo(helper, [a])

return local_rows.size

Expand Down

0 comments on commit 48dea18

Please sign in to comment.