Repository: projectmesa/mesa
Pull request #1788 (2023-09-03 18:31:12)

Add UserInputs component
Upgraded the make_user_input function into a solara component. 
Updated the model parameters handling in general
Reordered the JupyterViz Component: Now all UI elements are rendered at the bottom of the function. 

Corvince: oopsie thanks to @rebeccadavidsson we had tests for make_user_params! Updated the first test right now, leaving the others for tomorrow 
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/projectmesa/mesa/pull/1788?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=projectmesa) Report
Patch coverage: **`42.30%`** and project coverage change: **`-0.07%`** :warning:
> Comparison is base [(`47637a7`)](https://app.codecov.io/gh/projectmesa/mesa/commit/47637a7edd391678a4102aba573bc2160c8b7979?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=projectmesa) 79.88% compared to head [(`541442c`)](https://app.codecov.io/gh/projectmesa/mesa/pull/1788?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=projectmesa) 79.81%.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             main    #1788      +/-   ##
==========================================
- Coverage   79.88%   79.81%   -0.07%     
==========================================
  Files          15       15              
  Lines         880      887       +7     
  Branches      188      190       +2     
==========================================
+ Hits          703      708       +5     
- Misses        154      156       +2     
  Partials       23       23              
```


| [Files Changed](https://app.codecov.io/gh/projectmesa/mesa/pull/1788?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=projectmesa) | Coverage Δ | |
|---|---|---|
| [mesa/experimental/jupyter\_viz.py](https://app.codecov.io/gh/projectmesa/mesa/pull/1788?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=projectmesa#diff-bWVzYS9leHBlcmltZW50YWwvanVweXRlcl92aXoucHk=) | `21.27% <42.30%> (+2.61%)` | :arrow_up: |


</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/projectmesa/mesa/pull/1788?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=projectmesa).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=projectmesa).

Corvince: Added missing tests and removed the global state - thx @rht for the heads-up
Corvince: I updated and simplified the change_handler. Would this be sufficient @rht or do you need anything else for this PR to be mergeable?
rht: I almost agree with your change to make it one-way here. Also that `handle_change_model_params` is a thin wrapper of `set_model_parameters`, similar to the inverse data flow recommendation in https://react.dev/learn/thinking-in-react#step-5-add-inverse-data-flow.

If you can explain why we shouldn't use `use_reactive` and I am convinced, then I will merge:
Solara's `use_reactive` doesn't have an equivalent API in React. It seems to be a convenient API sugar specific to Solara. It allows child components to modify the parent state directly. The way for the parent component to be notified of the change event is to specify `on_change` during initialization. This works. And so what is the downside of this? The state is still defined in the parent scope, and the updater is passed down from the parent, just like `set_model_parameters`.
Corvince: Thanks for your swift reply and clarification. 

And I feel like I might be missing something super obvious. But I have to ask you again, because I still don't get it. 

For which variable should `use_reactive` be used? 
rht: I meant the subset of the model parameters that are meant to be changed by `UserInputs`, as in the current main.

But according to Solara dev (https://discord.com/channels/1106593685241614489/1106593686223069309/1138104278792290355), `use_reactive` is not meant to be defined inside a for loop or conditional. So for this reason alone, your solution is more favorable, unless you know a simple way to use 1 `use_reactive` without a for loop (e.g. passing the whole dict as a state).

Patch:
diff --git a/mesa/experimental/jupyter_viz.py b/mesa/experimental/jupyter_viz.py
index 93b0d852..fd2d045e 100644
--- a/mesa/experimental/jupyter_viz.py
+++ b/mesa/experimental/jupyter_viz.py
@@ -39,25 +39,26 @@ def JupyterViz(
 
     current_step, set_current_step = solara.use_state(0)
 
-    solara.Markdown(name)
-
-    # 0. Split model params
-    model_params_input, model_params_fixed = split_model_params(model_params)
-
-    # 1. User inputs
-    user_inputs = {}
-    for name, options in model_params_input.items():
-        user_input = solara.use_reactive(options["value"])
-        user_inputs[name] = user_input.value
-        make_user_input(user_input, name, options)
+    # 1. Set up model parameters
+    user_params, fixed_params = split_model_params(model_params)
+    model_parameters, set_model_parameters = solara.use_state(
+        fixed_params | {k: v["value"] for k, v in user_params.items()}
+    )
 
-    # 2. Model
+    # 2. Set up Model
     def make_model():
-        return model_class(**user_inputs, **model_params_fixed)
+        model = model_class(**model_parameters)
+        set_current_step(0)
+        return model
 
-    model = solara.use_memo(make_model, dependencies=list(user_inputs.values()))
+    model = solara.use_memo(make_model, dependencies=list(model_parameters.values()))
 
-    # 3. Buttons
+    def handle_change_model_params(name: str, value: any):
+        set_model_parameters(model_parameters | {name: value})
+
+    # 3. Set up UI
+    solara.Markdown(name)
+    UserInputs(user_params, on_change=handle_change_model_params)
     ModelController(model, play_interval, current_step, set_current_step)
 
     with solara.GridFixed(columns=2):
@@ -160,44 +161,53 @@ def check_param_is_fixed(param):
         return True
 
 
-def make_user_input(user_input, name, options):
-    """Initialize a user input for configurable model parameters.
+@solara.component
+def UserInputs(user_params, on_change=None):
+    """Initialize user inputs for configurable model parameters.
     Currently supports :class:`solara.SliderInt`, :class:`solara.SliderFloat`,
     and :class:`solara.Select`.
 
-    Args:
-        user_input: :class:`solara.reactive` object with initial value
-        name: field name; used as fallback for label if 'label' is not in options
-        options: dictionary with options for the input, including label,
+    Props:
+        user_params: dictionary with options for the input, including label,
         min and max values, and other fields specific to the input type.
+        on_change: function to be called with (name, value) when the value of an input changes.
     """
-    # label for the input is "label" from options or name
-    label = options.get("label", name)
-    input_type = options.get("type")
-    if input_type == "SliderInt":
-        solara.SliderInt(
-            label,
-            value=user_input,
-            min=options.get("min"),
-            max=options.get("max"),
-            step=options.get("step"),
-        )
-    elif input_type == "SliderFloat":
-        solara.SliderFloat(
-            label,
-            value=user_input,
-            min=options.get("min"),
-            max=options.get("max"),
-            step=options.get("step"),
-        )
-    elif input_type == "Select":
-        solara.Select(
-            label,
-            value=options.get("value"),
-            values=options.get("values"),
-        )
-    else:
-        raise ValueError(f"{input_type} is not a supported input type")
+
+    for name, options in user_params.items():
+        # label for the input is "label" from options or name
+        label = options.get("label", name)
+        input_type = options.get("type")
+
+        def change_handler(value, name=name):
+            on_change(name, value)
+
+        if input_type == "SliderInt":
+            solara.SliderInt(
+                label,
+                value=options.get("value"),
+                on_value=change_handler,
+                min=options.get("min"),
+                max=options.get("max"),
+                step=options.get("step"),
+            )
+        elif input_type == "SliderFloat":
+            solara.SliderFloat(
+                label,
+                value=options.get("value"),
+                on_value=change_handler,
+                min=options.get("min"),
+                max=options.get("max"),
+                step=options.get("step"),
+            )
+        elif input_type == "Select":
+            solara.Select(
+                label,
+                value=options.get("value"),
+                on_value=change_handler,
+                values=options.get("values"),
+            )
+        else:
+            raise ValueError(f"{input_type} is not a supported input type")
 
 
 def make_space(model, agent_portrayal):


Test patch:
diff --git a/tests/test_jupyter_viz.py b/tests/test_jupyter_viz.py
index 4e26d569..f1d7371c 100644
--- a/tests/test_jupyter_viz.py
+++ b/tests/test_jupyter_viz.py
@@ -1,53 +1,71 @@
 import unittest
 from unittest.mock import Mock, patch
 
+import ipyvuetify as vw
 import solara
 
-from mesa.experimental.jupyter_viz import JupyterViz, make_user_input
+from mesa.experimental.jupyter_viz import JupyterViz, UserInputs
 
 
 class TestMakeUserInput(unittest.TestCase):
     def test_unsupported_type(self):
+        @solara.component
+        def Test(user_params):
+            UserInputs(user_params)
+
         """unsupported input type should raise ValueError"""
         # bogus type
         with self.assertRaisesRegex(ValueError, "not a supported input type"):
-            make_user_input(10, "input", {"type": "bogus"})
+            solara.render(Test({"mock": {"type": "bogus"}}), handle_error=False)
+
         # no type is specified
         with self.assertRaisesRegex(ValueError, "not a supported input type"):
-            make_user_input(10, "input", {})
+            solara.render(Test({"mock": {}}), handle_error=False)
+
+    def test_slider_int(self):
+        @solara.component
+        def Test(user_params):
+            UserInputs(user_params)
 
-    @patch("mesa.experimental.jupyter_viz.solara")
-    def test_slider_int(self, mock_solara):
-        value = 10
-        name = "num_agents"
         options = {
             "type": "SliderInt",
+            "value": 10,
             "label": "number of agents",
             "min": 10,
             "max": 20,
             "step": 1,
         }
-        make_user_input(value, name, options)
-        mock_solara.SliderInt.assert_called_with(
-            options["label"],
-            value=value,
-            min=options["min"],
-            max=options["max"],
-            step=options["step"],
-        )
+        user_params = {"num_agents": options}
+        _, rc = solara.render(Test(user_params), handle_error=False)
+        slider_int = rc.find(vw.Slider).widget
+
+        assert slider_int.v_model == options["value"]
+        assert slider_int.label == options["label"]
+        assert slider_int.min == options["min"]
+        assert slider_int.max == options["max"]
+        assert slider_int.step == options["step"]
 
-    @patch("mesa.experimental.jupyter_viz.solara")
-    def test_label_fallback(self, mock_solara):
+    def test_label_fallback(self):
         """name should be used as fallback label"""
-        value = 10
-        name = "num_agents"
+
+        @solara.component
+        def Test(user_params):
+            UserInputs(user_params)
+
         options = {
             "type": "SliderInt",
+            "value": 10,
         }
-        make_user_input(value, name, options)
-        mock_solara.SliderInt.assert_called_with(
-            name, value=value, min=None, max=None, step=None
-        )
+
+        user_params = {"num_agents": options}
+        _, rc = solara.render(Test(user_params), handle_error=False)
+        slider_int = rc.find(vw.Slider).widget
+
+        assert slider_int.v_model == options["value"]
+        assert slider_int.label == "num_agents"
+        assert slider_int.min is None
+        assert slider_int.max is None
+        assert slider_int.step is None
 
 
 class TestJupyterViz(unittest.TestCase):