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

[Request for input] Keyed widget state persistence - discussion, possible fixes #6074

Open
sfc-gh-jcarroll opened this issue Feb 7, 2023 · 8 comments
Labels
area:widgets feature:st.session_state type:enhancement Requests for feature enhancements or new features

Comments

@sfc-gh-jcarroll
Copy link
Collaborator

sfc-gh-jcarroll commented Feb 7, 2023

👋 I work on developer experience for Streamlit and wanted to get some feedback on this recurring issue.

TL;DR: Take a look at the example app and let us know how it impacts you, and what you think about how to fix this issue!

Discussion on the forums here

One of the current Streamlit behaviors that causes confusion for many of us is how widgets with key='foo' lose their session_state value when they are not rendered in a given run. It seems to come up most often with multipage apps that have some state dependency between pages, but also possible in a single page app with conditional rendering. A minimal example:

view = st.radio("View", ["view1", "view2"])

if view == "view1":
    st.text_input("Text1", key="text1")
    "☝️ Enter some text, then click on view2 above"
elif view == "view2":
    "☝️ Now go back to view1 and see if your text is still there"

I'm working with @tconkling on whether / how to update this behavior so it's more intuitive, or at least possible to get the intuitive behavior easily.

I put together a simple example app showing the behavior and linking some of the prior issues about it:
https://keyed-widget-state-issue.streamlit.app/

Questions

  • Do you have a full working public app where you had to hack around this (such as duplicating or using "shadow keys")? Please share with us!
  • Do you have an app or use case where you rely on the current behavior (widget has key set but needs value to reset after not rendering), that will break if we change this? We really want to hear about these! Please share!

Possible fixes

It seems like most developers we hear from find the current behavior very unintuitive, and we see a lot more use cases that have to work around this rather than benefit from it.

  • Given that, should we just do a breaking change to the default behavior to persist, like most of us seem to expect?
  • If so: We should still provide a path to clear keyed widget state when needed. Need to figure out how to best do this.

If not a breaking change: most proposals seem to prefer something like a persist= key. For example, both @whitphx and @MathCatsAnd propose something like this in #4458. Thoughts on this approach, or other ideas?


Community voting on feature requests enables the Streamlit team to understand which features are most important to our users.

If you'd like the Streamlit team to prioritize this feature request, please use the 👍 (thumbs up emoji) reaction in response to the initial post.

@devxpy
Copy link

devxpy commented Feb 8, 2023

Thanks for throwing shade at this issue!

I've built a really complex app with streamlit, and have been stung time and again due to this behaviour. See for e.g. - https://gooey.ai/video-bots/


Currently using the following workaround for this -

  1. Don't use keys. Instead, store the initial value inside the session state, and update the state manually.

  2. If the widget goes out of view, make sure the initial values are up to date

from time import sleep
from streamlit.runtime.state import get_session_state
import streamlit as st

init_key = "__init_values__"


def main():
    view = st.radio("View", ["view1", "view2"])

    if view == "view1":
        text_input("Text1", key="text1")
        "☝️ Enter some text, then click on view2 above"
    elif view == "view2":
        "☝️ Now go back to view1 and see if your text is still there"

    st.write(st.session_state)


def text_input(label, key):
    if init_key not in st.session_state:
        st.session_state[init_key] = {}
    initial_values = st.session_state[init_key]
    try:
        value = initial_values[key]
    except KeyError:
        value = st.session_state.get(key, "")
        initial_values[key] = value

    value = st.text_input(label, value=value)

    st.session_state[key] = value
    return value


def ensure_hidden_widgets_loaded():
    new_session_state = get_session_state()._state._new_session_state
    for key, value in st.session_state.items():
        if key in new_session_state or key == init_key:
            continue
        st.session_state[init_key][key] = value


try:
    main()
finally:
    ensure_hidden_widgets_loaded()

IMO the idea of keys, in general, needs to be rethought. They cause really hard-to-debug issues.

E.g. see how the slider widget behaves when provided with a key. And also how text editing is jumpy when a piece of blocking code is executed.

But permanently changing this behaviour would be an excellent start. For cases where developers need to clear widget state, there should be a clear path to do it explicitly, e.g. st.session_state.pop(key, None) or st.session_state.clear()

@whitphx
Copy link
Contributor

whitphx commented Feb 9, 2023

Hi I am the author of #4458.
Let me just provide supplement information, while it's not any additional examples or suggestions.

Even though I introduced a multi page-like example in the issue above, it does not imply the persist API (or some alternatives) should work across the pages on the native MPA. My interest at that point was only the state of unrendered components in a single page.
When I wrote the issue was before the release of native multi-page apps support (or at least before I started using it), so it accidentally looked like MPA.

And the MPA-like example was just a simplified example to explain the problem; Originally I encountered some difficulty derived from this problem when I was developing a custom component streamlit-webrtc, and did workaround somehow.

@sfc-gh-jcarroll
Copy link
Collaborator Author

Thanks, all!

E.g. see how the slider widget behaves when provided with a key. And also how text editing is jumpy when a piece of blocking code is executed.

Was talking with @tconkling about some of these funky behaviors today, definitely want to fix those!

it does not imply the persist API (or some alternatives) should work across the pages on the native MPA.

Yeah I think I am coming around to this perspective too. Was discussing an alternative idea of clearer scopes between pages for MPA here. Wonder if that resonates or if you have any thoughts.

@MartinNowak
Copy link

One of the current Streamlit behaviors that causes confusion for many of us is how widgets with key='foo' lose their session_state value when they are not rendered in a given run

Well in fact their widget state gets deleted while session state is persisted across re-runs.
Given how this distinction is already exposed, it could make be left to users to decide whether or not to preserve widget state across reruns.
Given that this tends to affect more complicated apps, I would presume we mostly wouldn't even mind a component-levelpersist_state=True flag. An app-wide setting would be an improvement without polluting the API.

As a catch with app-wide behavior, conditionally rendered widgets can have different valid options/number ranges when being rendered again, possibly making the persisted state now invalid.

@sfc-gh-jcarroll
Copy link
Collaborator Author

As a catch with app-wide behavior, conditionally rendered widgets can have different valid options/number ranges when being rendered again, possibly making the persisted state now invalid.

Yes, if the widget arguments change, we will always re-create the widget and clear any prior values/state. I don't think there's any consideration to change that behavior, since as you said it can lead to invalid states pretty easily.

@jelledijkstra97
Copy link

👍

Great to read this thread. Although I really like Streamlit this behaviour makes it pretty much impossible to use it when working with a lot of filter settings. A ‘persist=‘ flag would work, but I prefer it with True as default.

@GitMarco27
Copy link

Hi there! I'm excited to be contributing to this project and I hope my contribution is helpful. I wanted to report an issue that I came across and I hope I'm reporting it in the right place. Please let me know if there's anything else I can do to help. Thanks!

I was able to perform a complete serialization of my application (for the components I needed), by means of the following method, based on what @devxpy reported, which I thank.

from __future__ import annotations
import streamlit as st


def __get_actual_page_ui_state():
    return st.session_state["session_manager"].ui_state[st.session_state['ActualPage']]


def gus():
    return __get_actual_page_ui_state()


__shadow_key_character = "_"


def __get_component(component, label: str, key: str, value, **kwargs):
    shadow_key = __shadow_key_character + key
    ui_state = gus()
    if shadow_key not in st.session_state:
        if component == st.selectbox:
            options = kwargs["options"]
            if key in ui_state:
                if ui_state[key] < len(options):
                    st.session_state[shadow_key] = options[ui_state[key]]
            else:
                if value < len(options):
                    st.session_state[shadow_key] = options[value]
        else:
            st.session_state[shadow_key] = ui_state.get(key, value)
    if component == st.multiselect:
        actual_value = component(label,
                                 key=shadow_key,
                                 default=value,
                                 **kwargs)
        ui_state[key] = actual_value
    elif component == st.selectbox:
        options = kwargs["options"]
        if value > len(options):
            value = 0
        actual_value = component(label=label,
                                 key=shadow_key,
                                 index=int(value),
                                 **kwargs)
        if actual_value is not None:
            ui_state[key] = options.index(actual_value)
    else:
        actual_value = component(label,
                                 key=shadow_key,
                                 value=value,
                                 **kwargs)
        ui_state[key] = actual_value
    return actual_value


def text_input(label: str, key: str, value, **kwargs):
    return __get_component(st.text_input, label, key, value, **kwargs)


def number_input(label: str, key: str, value, **kwargs):
    return __get_component(st.number_input, label, key, value, **kwargs)


def checkbox(label: str, key: str, value, **kwargs):
    return __get_component(st.checkbox, label, key, value, **kwargs)


def slider(label: str, key: str, value, **kwargs):
    return __get_component(st.slider, label, key, value, **kwargs)


def multiselect(label: str, key: str, default, **kwargs):
    return __get_component(component=st.multiselect, label=label,
                           key=key,
                           value=default,
                           **kwargs)


def selectbox(label: str, key: str, index, **kwargs):
    return __get_component(component=st.selectbox,
                           label=label,
                           key=key,
                           value=index,
                           **kwargs)

In order to make the code work within my multi-page application, I had to add the following code, which is executed every time the current page is changed, to handle the fact that for some components (multiselectbox) the key is not deleted.

# Clearing residuals shadow keys not removed by streamlit
# This is necessary because of the actual implementation of shadow keys
# for UI serialization
for key in st.session_state:
    if key[0] == cr.__shadow_key_character:
        del st.session_state[key]

@dpinol
Copy link

dpinol commented Jun 28, 2023

Hi,
any news on this? I'm not yet using streamlit, and hence I can't give any feedback.
What surprise me is that this issue does not appear in streamlit's roadmap

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:widgets feature:st.session_state type:enhancement Requests for feature enhancements or new features
Projects
None yet
Development

No branches or pull requests

8 participants