Skip to content

Commit

Permalink
fix: allow usage of use_reactive from outside kernel_context (#952)
Browse files Browse the repository at this point in the history
If the reactive variable return from use_reactive is set from outside
a kernel_context, the reactive variable would not be set.

This bug is demonstrated in:
  https://py.cafe/maartenbreddels/solara-shared-state

Where we had to force use_trait_observe to use use_state in of
use_reactive.
  • Loading branch information
maartenbreddels authored Dec 24, 2024
1 parent 1f6d98e commit db4435e
Show file tree
Hide file tree
Showing 4 changed files with 220 additions and 6 deletions.
114 changes: 111 additions & 3 deletions solara/_stores.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,16 @@
import copy
import dataclasses
import inspect
from typing import Callable, ContextManager, Generic, Optional, Union, cast
import sys
import threading
from typing import Callable, ContextManager, Generic, Optional, Union, cast, Any
import warnings
from .toestand import ValueBase, KernelStore, S, _find_outside_solara_frame


from .toestand import ValueBase, S, _find_outside_solara_frame, _DEBUG

import solara.util
import solara.settings


class _PublicValueNotSet:
Expand All @@ -25,7 +31,7 @@ class StoreValue(Generic[S]):


class MutateDetectorStore(ValueBase[S]):
def __init__(self, store: KernelStore[StoreValue[S]], equals=solara.util.equals_extra):
def __init__(self, store: ValueBase[StoreValue[S]], equals=solara.util.equals_extra):
self._storage = store
self._enabled = True
super().__init__(equals=equals)
Expand Down Expand Up @@ -187,3 +193,105 @@ def listener_wrapper(new: StoreValue[S], previous: StoreValue[S]):
listener(new_value, previous_value)

return self._storage.subscribe_change(listener_wrapper, scope=scope)


class SharedStore(ValueBase[S]):
"""Stores a single value, not kernel scoped."""

_traceback: Optional[inspect.Traceback]
_original_ref: Optional[S]
_original_ref_copy: Optional[S]

def __init__(self, value: S, equals: Callable[[Any, Any], bool] = solara.util.equals_extra, unwrap=lambda x: x):
# since a set can trigger events, which can trigger new updates, we need a recursive lock
self._lock = threading.RLock()
self.local = threading.local()
self.equals = equals

self._value = value
self._original_ref = None
self._original_ref_copy = None
self._unwrap = unwrap
self._mutation_detection = solara.settings.storage.mutation_detection
if self._mutation_detection:
frame = _find_outside_solara_frame()
if frame is not None:
self._traceback = inspect.getframeinfo(frame)
else:
self._traceback = None
self._original_ref = value
self._original_ref_copy = copy.deepcopy(self._original_ref)
if not self.equals(self._unwrap(self._original_ref), self._unwrap(self._original_ref_copy)):
msg = """The equals function for this reactive value returned False when comparing a deepcopy to itself.
This reactive variable will not be able to detect mutations correctly, and is therefore disabled.
To avoid this warning, and to ensure that mutation detection works correctly, please provide a better equals function to the reactive variable.
A good choice for dataframes and numpy arrays might be solara.util.equals_pickle, which will also attempt to compare the pickled values of the objects.
Example:
df = pd.DataFrame({"a": [1, 2, 3], "b": [4, 5, 6]})
reactive_df = solara.reactive(df, equals=solara.util.equals_pickle)
"""
tb = self._traceback
if tb:
if tb.code_context:
code = tb.code_context[0]
else:
code = "<No code context available>"
msg += "This warning was triggered from:\n" f"{tb.filename}:{tb.lineno}\n" f"{code}"
warnings.warn(msg)
self._mutation_detection = False
super().__init__(equals=equals)

def _check_mutation(self):
if not self._mutation_detection:
return
current = self._unwrap(self._original_ref)
initial = self._unwrap(self._original_ref_copy)
if not self.equals(initial, current):
tb = self._traceback
if tb:
if tb.code_context:
code = tb.code_context[0].strip()
else:
code = "No code context available"
msg = f"Reactive variable was initialized at {tb.filename}:{tb.lineno} with {initial!r}, but was mutated to {current!r}.\n" f"{code}"
else:
msg = f"Reactive variable was initialized with a value of {initial!r}, but was mutated to {current!r} (unable to report the location in the source code)."
raise ValueError(msg)

@property
def lock(self):
return self._lock

def peek(self):
self._check_mutation()
return self._value

def get(self):
self._check_mutation()
return self._value

def clear(self):
pass

def _get_scope_key(self):
return "GLOBAL"

def set(self, value: S):
self._check_mutation()
old = self.get()
if self.equals(old, value):
return
self._value = value

if _DEBUG:
import traceback

traceback.print_stack(limit=17, file=sys.stdout)

print("change old", old) # noqa
print("change new", value) # noqa

self.fire(value, old)
17 changes: 16 additions & 1 deletion solara/hooks/use_reactive.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from typing import Any, Callable, Optional, TypeVar, Union

import solara
import solara.settings

T = TypeVar("T")

Expand Down Expand Up @@ -105,7 +106,21 @@ def MyComponent(value: Union[T, solara.Reactive[T]],

def create():
if not isinstance(value, solara.Reactive):
return solara.reactive(value)
from solara._stores import SharedStore, MutateDetectorStore, StoreValue, _PublicValueNotSet, _SetValueNotSet
from solara.toestand import ValueBase

store: ValueBase[T]

if solara.settings.storage.mutation_detection is True:
shared_store = SharedStore[StoreValue[T]](
StoreValue[T](private=value, public=_PublicValueNotSet(), get_traceback=None, set_value=_SetValueNotSet(), set_traceback=None),
unwrap=lambda x: x.private,
)
store = MutateDetectorStore[T](shared_store, equals=equals)
else:
store = SharedStore(value, equals=equals)

return solara.Reactive(store)

reactive_value = solara.use_memo(create, dependencies=[])
if isinstance(value, solara.Reactive):
Expand Down
6 changes: 5 additions & 1 deletion solara/toestand.py
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,9 @@ def setter(new_value: TS):

return cast(Callable[[TS], None], setter)

def _check_mutation(self):
pass


# the default store for now, stores in a global dict, or when in a solara
# context, in the solara user context
Expand Down Expand Up @@ -316,7 +319,8 @@ def _is_internal_module(file_name: str):
return (
file_name_parts[-2:] == ["solara", "toestand.py"]
or file_name_parts[-2:] == ["solara", "reactive.py"]
or file_name_parts[-2:] == ["solara", "use_reactive.py"]
or file_name_parts[-2:] == ["solara", "_stores.py"]
or file_name_parts[-3:] == ["solara", "hooks", "use_reactive.py"]
or file_name_parts[-2:] == ["reacton", "core.py"]
# If we use SomeClass[K](...) we go via the typing module, so we need to skip that as well
or (file_name_parts[-2].startswith("python") and file_name_parts[-1] == "typing.py")
Expand Down
89 changes: 88 additions & 1 deletion tests/unit/toestand_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,47 @@ def test_scopes(no_kernel_context):
for u in unsub:
u()

# make sure the use_reactive can be used outside of a kernel context
reactives = [None, None]
index = 0

@solara.component
def Page():
reactive = solara.use_reactive(0)
if reactives[index] is None:
reactives[index] = reactive # type: ignore
solara.Info(str(reactive.value))

with context1:
_, rc1 = react.render(Page(), handle_error=False)
assert rc1.find(v.Alert).widget.children[0] == "0"
assert reactives[0] is not None

with context2:
index = 1
_, rc2 = react.render(Page(), handle_error=False)
assert rc2.find(v.Alert).widget.children[0] == "0"
assert reactives[1] is not None

reactives[0].value = 1
assert rc1.find(v.Alert).widget.children[0] == "1"
assert rc2.find(v.Alert).widget.children[0] == "0"

reactives[1].value = 2
assert rc1.find(v.Alert).widget.children[0] == "1"
assert rc2.find(v.Alert).widget.children[0] == "2"

assert reactives[0].value == 1
assert reactives[1].value == 2
with context1:
assert reactives[0].value == 1
assert reactives[1].value == 2

with context1:
rc1.close()
with context2:
rc2.close()


def test_nested_update():
# this effectively test the RLock vs Lock
Expand Down Expand Up @@ -1270,7 +1311,6 @@ def test_mutate_initial_value(no_kernel_context, mutation_detection):
initial_values: List[int] = [1, 2, 3]
reactive = Reactive[List[int]](solara.toestand.mutation_detection_storage(initial_values))
assert reactive.value == initial_values
# breakpoint()
initial_values.append(4)
with pytest.raises(ValueError, match="Reactive variable was initialized"):
reactive.value = [3, 2, 1]
Expand All @@ -1283,14 +1323,61 @@ def test_mutate_initial_value(no_kernel_context, mutation_detection):
_ = reactive.value


@pytest.mark.skipif(not solara.settings.storage.mutation_detection, reason="only tests when SOLARA_STORAGE_MUTATION_DETECTION=1")
def test_mutate_initial_value_local(no_kernel_context, mutation_detection):
initial_values: List[int] = [1, 2, 3]
reactive_local = solara.reactive([4, 5, 6])

@solara.component
def Page():
nonlocal reactive_local
reactive_local = solara.use_reactive(initial_values)

_, rc = react.render(Page(), handle_error=False)
try:
assert reactive_local.value == initial_values
initial_values.append(4)
with pytest.raises(ValueError, match="Reactive variable was initialized"):
reactive_local.value = [3, 2, 1]
kernel_shared = kernel.Kernel()
context = kernel_context.VirtualKernelContext(id="toestand-1", kernel=kernel_shared, session_id="session-1")
assert kernel_context.current_context[kernel_context.get_current_thread_key()] is None
with pytest.raises(ValueError, match="Reactive variable was initialized"):
with context:
# reading the initial values should also trigger the error
_ = reactive_local.value
finally:
rc.close()


def test_mutate_value_public_value():
values = [1, 2, 3]

reactive = Reactive[List[int]](solara.toestand.mutation_detection_storage(values))
reactive.value.append(4)
with pytest.raises(ValueError, match="Reactive variable was read when it had the value of \\[1, 2, 3\\].*"):
reactive._storage.check_mutations() # type: ignore


@pytest.mark.skipif(not solara.settings.storage.mutation_detection, reason="only tests when SOLARA_STORAGE_MUTATION_DETECTION=1")
def test_mutate_value_public_value_local():
values = [1, 2, 3]
reactive = solara.reactive(values)

@solara.component
def Page():
nonlocal reactive
reactive = solara.use_reactive(values)

_, rc = react.render(Page(), handle_error=False)
try:
reactive.value.append(4)
with pytest.raises(ValueError, match="Reactive variable was read when it had the value of \\[1, 2, 3\\].*"):
_ = reactive.value
finally:
rc.close()


def test_mutate_value_set_value():
values = [1, 2, 3]
reactive = Reactive[List[int]](solara.toestand.mutation_detection_storage(values))
Expand Down

0 comments on commit db4435e

Please sign in to comment.