444B / streamlit-analytics2

👀 Track & visualize user interactions with your streamlit app
MIT License
13 stars 2 forks source link

[BUG] Checkbox counts are incremented on startup #102

Open robino16 opened 1 week ago

robino16 commented 1 week ago

Describe the bug The counts for all checkboxes are incremented on startup without user interaction. Counts for checkboxes always start at one instead of zero and are incremented without interaction from the user.

Expected behavior I would expect the counts for all widgets to start at zero the first time the app is run. Once a user checks or unchecks a checkbox, the count for that checkbox should increment by one. Checkboxes that are untouched should remain at count 0.

To Reproduce

  1. Make a Streamlit app with at least one checkbox (and which stores the counts in, e.g., a JSON file).
  2. Run the app.
  3. See that the count for all checboxes start at 1 instead of 0 by examining the JSON file.
  4. Stop and re-start the app.
  5. See that the count for all checkboxes have been incremented by 1 without user interaction.

Software Vesions

Additional context The original streamlit-analytics repo by jrieke also has this bug.

Suggested fix: We should not increment the count the first time the script is run, only after a user has interacted with the checkbox. The following code attempts to solve this issue:

def _wrap_checkbox(func):
    """
    Wrap st.checkbox.
    """

    def new_func(label, *args, **kwargs):
        checked = func(label, *args, **kwargs)
        label = replace_empty(label)
        if label not in counts["widgets"]:
            counts["widgets"][label] = 0

        # we only increment the count if the user has interacted with the checbox
        if label in st.session_state.state_dict and checked != st.session_state.state_dict[label]:
            counts["widgets"][label] += 1

        st.session_state.state_dict[label] = checked
        return checked

    return new_func

The code and comments here are just suggestions.

444B commented 1 week ago

Thank you @robino16 for this issue! I truly enjoy the detail and formatting of these bugs :)

I will fix this and then also validate the startup/increment behavior of all widgets to ensure correct behavior

444B commented 1 week ago

Additionally, I am more than happy to accept a PR on this and add you as a collaborator to the project

444B commented 1 week ago

Initial findings:
This auto-incrementing issue is not limited to st.checkbox. Below we find a list of other inputs that also automatically increment on each successive page load:

    st.checkbox
    st.radio
    st..selectbox
    st.slider
    st.slider
    st.select_slider
    st.text_input
    st.number_input
    st.text_area
    st.date_input
    st.color_picker
    st.chat_input

Some of these makes sense since a default is being put as a value (such as st.color_picker) but where this behavior is not WAX is with st.checkbox, which will increment even if nothing is selected. There is a reason for this, detailed below.
I have looked into the docs for st.checkbox and my theory is that because st.checkbox is returning a boolean, it is still counted as an interaction even if the box was not ticked. It is incrementing that the box was NOT checked.

What I believe is a suitable solution is to provide more granularity on the usage of st.checkbox and show how many times it was False(unchecked) and True(checked) since right now the number shown in "widget usage" is counting both. That, or to implement your suggestion of only counting the True/checked usage.

The other widget types I have looked into briefly and attempted some mitigations which did not work. Putting a placeholder for st.text_input, st.number_input and st.text_area does not solve the issue since it still seems to be returning a value and being counted as an interaction and thus incremented.

I think this is a pretty important finding, thank you @robino16 for raising! The deeper issue here is that the data tracked by certain widgets is not reliable and may not reflect actual widget usage but only page loads.

robino16 commented 1 week ago

Hi, @444B .

I suspected that this issue could be present in more of the Streamlit components. Thank you for looking into it!

The wrapper functions of streamlit-analytics will run independent on the value of the component. And they will run every time the user perfoms a action in the UI. That's why even False values trigger the wrapper function for st.checkbox.

I believe that the original author of streamlit-analytics (jrieke) intended the counts for st.checkbox to be incremeneted every time the user checks or unchecks the checkbox, not only capturing the True/checked usage. The only problem is that the initial state also increments its value, which does seem a bit weird. My code suggestion attempts to fix the issue with st.checkbox by simply not counting the initial state, but it will increment the count every time the user checks or unchecks the checkbox.

How the counts should be measured are kind off a big question. Should the initial state increment the count? This means that the default states, like empty string " " for st.text_input will often have a high count number. This is quite obvious in the Live Demo of the original streamlit-analytics repo:

bilde

Here the default of st.text_input " " and default of the dropdown menu "cat" sticks out simply because they are the default values. This may be the desired behaviour, but for the st.checkbox in particular, which doesn't distinguish between False and True this makes less sense.

So I think you're right about the st.checkbox is missing the distinction between True and False counts. Adding more granularity to checkboxes and counting True vs. False values independently is a great idea! However, implementing this will likely reset the checkbox counts for all users of this library, which can be unfortunate.

I am not sure how to proceed from here. Implementing my code suggestion will change the behavior of st.checkbox so it only captures user interactions with the checkbox. Distinguishing between False and True may be a better option, but it may be more complex and is not backwards compatible.