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

Batch update: automatically? #637

Open
maartenbreddels opened this issue May 8, 2024 · 2 comments
Open

Batch update: automatically? #637

maartenbreddels opened this issue May 8, 2024 · 2 comments
Milestone

Comments

@maartenbreddels
Copy link
Contributor

Current situation

Currently, doing two state changes in a callback, will result in two render passes. If rendering is seen as a side-effect, rendering should only happen after the event handler is finished.

The workaround to get batch updates in event handlers for now is: (not a public solara API, might break in the future):

import solara
import reacton.core


text = solara.reactive("Initial text")

@solara.component
def Page():
    rc = reacton.core.get_render_context()
    print("Render with: ", text.value)
    def change_text():
        with rc:
            text.value = "Should never be rendered visible"
            text.value = "Updated text"
        
    solara.Button("Change text", on_click=change_text)
    solara.Text(text.value)

This has the upside of being explicit and allowing people to opt out (by simply not doing this).

Making this the default in Solara 2.0?

If we make batch updates in event handlers the default behaviour, it would be difficult to opt out (I cannot think of a good name or a way to signal this as a user).

Better API in 2.0?

Instead of making it a default and not easy to use (you have to get a handle to the render-context from the main render loop), we can think of a solara.batch_update() function/context-manager that more explicitly conveys intent.

import solara


text = solara.reactive("Initial text")

@solara.component
def Page():
    print("Render with: ", text.value)
    def change_text():
        with solara.batch_update:
            text.value = "Should never be rendered visible"
            text.value = "Updated text"
        
    solara.Button("Change text", on_click=change_text)
    solara.Text(text.value)

Consistency with tasks

If callbacks do automatic batch updating, people might expect a task to do the same. Should we even allow people to update reactive variables from a task, or should we only allow a return value (force them to be pure)? The progress value would then be an exception to this, since that is typically updated in a loop:

    import time
    import solara
    from solara.lab import task


    @task
    def my_calculation():
        total = 0
        for i in range(10):
            # mutate progress in the task
            my_calculation.progress = (i + 1) * 10.0
            time.sleep(0.4)
            if not my_calculation.is_current():
                # A new call was made before this call was finished
                return
            total += i**2
        return total


    @solara.component
    def Page():
        solara.Button("Run calculation", on_click=my_calculation)
        solara.ProgressLinear(my_calculation.progress if my_calculation.pending else False)

        if my_calculation.finished:
            solara.Text(f"Calculation result: {my_calculation.value}")
        elif my_calculation.not_called:
            solara.Text("Click the button to fetch data")
@maartenbreddels maartenbreddels added this to the Solara 2.0 milestone May 8, 2024
@JovanVeljanoski
Copy link
Collaborator

This batch_update feature would be a game change. A massive upvote for this.

@Jhsmit
Copy link
Contributor

Jhsmit commented May 22, 2024

from The Zen of Python: "Explicit is better than implicit."
Which would argue in favor of an explicit batch_update. That being set, setting .value on a reactive also involves automagic, but there I think its more explicitly expected that settings a reactive value is no ordinary setattr operation.

I think if you make this default / automatic in callbacks it might be confusing as users could expect multiple renders from setting multiple reactive values.

wrt tasks, I do at the moment set reactives in the global scope from tasks, so that would break my use case but if things become clearer (or glitch-free, no illegal states/incompatible combination of interdependent reactive values) then I wouldn't mind refactoring that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants