Skip to content
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

fix: Tighten use_memo dependency types #356

Merged
merged 5 commits into from
Mar 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions docker/data/storage/notebooks/DEMO.md
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ def hist_demo(source, column):
x=column,
nbins=bin_count,
),
{bin_count, hist_range, source, column},
[bin_count, hist_range, source, column],
)

return [
Expand Down 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
Loading