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

Fix widget id stability #7003

Merged
merged 47 commits into from Jul 27, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
e87fb15
Add (failing) slider id stability test
AnOctopus Jul 11, 2023
62b5164
Hacky fix for slider id stability
AnOctopus Jul 11, 2023
335f829
Document safe dependency on state in slider
AnOctopus Jul 13, 2023
2d356e5
Crude early detection of common nonidentity fields
AnOctopus Jul 13, 2023
3908489
Add new widget id function
AnOctopus Jul 17, 2023
0520df1
Convert most widgets to new ID generation
AnOctopus Jul 17, 2023
6ecf282
Add docs for requirements for calling widget id gen
AnOctopus Jul 17, 2023
1a41980
Use current form id as widget id input
AnOctopus Jul 17, 2023
ffa2395
Merge branch 'develop' into fix/widget-id-stability
AnOctopus Jul 17, 2023
4edb863
Add missing label to time widget id
AnOctopus Jul 18, 2023
65f5df6
Early generate ids for custom components and file uploader
AnOctopus Jul 18, 2023
8be3255
Generate id for data editor
AnOctopus Jul 18, 2023
6fb2a1b
Fix and simplify keyed widget id test
AnOctopus Jul 18, 2023
460c3d0
Update other deltagen widget id tests
AnOctopus Jul 18, 2023
2e26e7b
Warn about fallback widget id generation
AnOctopus Jul 18, 2023
af91723
Concision
AnOctopus Jul 18, 2023
3c2dc38
Add widget id kwargs types, start resolving conflicts
AnOctopus Jul 19, 2023
be63d8a
Merge branch 'develop' into fix/widget-id-stability
AnOctopus Jul 19, 2023
699a703
Allow common types with safe str encodings
AnOctopus Jul 20, 2023
d9d0ca0
Finish hacky hashing of existing safe values
AnOctopus Jul 20, 2023
1969ffe
Fix import cycle
AnOctopus Jul 20, 2023
863d8f9
Merge branch 'develop' into fix/widget-id-stability
AnOctopus Jul 21, 2023
4b2e1ed
Merge branch 'develop' into fix/widget-id-stability
AnOctopus Jul 21, 2023
8e7c7d9
Clean up union in signature
AnOctopus Jul 24, 2023
ebe180a
Partial processing of time value
AnOctopus Jul 24, 2023
e30e3b2
Date parsing
AnOctopus Jul 24, 2023
0208861
Hash multiselct index for default
AnOctopus Jul 24, 2023
e9ecdb6
Add unresolved todo
AnOctopus Jul 24, 2023
d5ffbd3
Fix dumb errors
AnOctopus Jul 25, 2023
b828da2
Use parse function for min and max
AnOctopus Jul 25, 2023
12b1fc2
Rename old widget id computation function
AnOctopus Jul 25, 2023
12e0304
Rename new widget id function
AnOctopus Jul 25, 2023
2eef088
Document safety from ensured dict iter order
AnOctopus Jul 25, 2023
18ddcb7
Update custom components hashing comments
AnOctopus Jul 25, 2023
2cc6957
Add missing widget id computation do download button
AnOctopus Jul 25, 2023
7ed310b
Remove unneeded fallback id calculation
AnOctopus Jul 25, 2023
8f1fb45
Remove old widget id function
AnOctopus Jul 25, 2023
a9b008d
Comment about late id calculation in data_editor
AnOctopus Jul 25, 2023
481c339
Update slider session_state use documentation
AnOctopus Jul 25, 2023
27498d3
Try changing name to unconfuse codeql
AnOctopus Jul 25, 2023
19bb0bc
Remove dead imports
AnOctopus Jul 25, 2023
50f7b70
Avoid impossible uninitialized variable
AnOctopus Jul 25, 2023
1ad9c16
Use `disabled` in data editor id
AnOctopus Jul 26, 2023
0da4e02
Remove dead code
AnOctopus Jul 26, 2023
25aea34
Distinguish download button id
AnOctopus Jul 26, 2023
223b2e1
Use column_config_mapping
AnOctopus Jul 27, 2023
fe30d51
Use arrow_bytes
AnOctopus Jul 27, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
40 changes: 27 additions & 13 deletions lib/streamlit/elements/slider.py
Expand Up @@ -376,19 +376,6 @@ def _slider(

maybe_raise_label_warnings(label, label_visibility)

if value is None:
# Set value from session_state if exists.
session_state = get_session_state().filtered_state

# we look first to session_state value of the widget because
# depending on the value (single value or list/tuple) the slider should be
# initializing differently (either as range or single value slider)
if key is not None and key in session_state:
value = session_state[key]
else:
# Set value default.
value = min_value if min_value is not None else 0

SUPPORTED_TYPES = {
Integral: SliderProto.INT,
Real: SliderProto.FLOAT,
Expand All @@ -398,6 +385,33 @@ def _slider(
}
TIMELIKE_TYPES = (SliderProto.DATETIME, SliderProto.TIME, SliderProto.DATE)

if value is None:
# We need to know if this is a single or range slider, but don't have
# a default value. We will try looking at the value in session state,
# but we must be careful, because we don't want the slider's identity
# to be derived from the value, or it will change every time the value
# changes, which would be a problem if we stop garbage collecting
# widgets that haven't been used recently.

single_value = True

session_state = get_session_state().filtered_state

if key is not None and key in session_state:
state_value = session_state[key]
single_value = isinstance(state_value, tuple(SUPPORTED_TYPES.keys()))

# Set value default. We assume single vs range doesn't change in the
# lifetime of a slider, so it is safe to depend on, but we use the
# min/max values and constants for the actual default, to prevent
# depending on the value.
if single_value:
value = min_value if min_value is not None else 0
else:
mn = min_value if min_value is not None else 0
mx = max_value if max_value is not None else 100
value = [mn, mx]

# Ensure that the value is either a single value or a range of values.
single_value = isinstance(value, tuple(SUPPORTED_TYPES.keys()))
range_value = isinstance(value, (list, tuple)) and len(value) in (0, 1, 2)
Expand Down
20 changes: 20 additions & 0 deletions lib/streamlit/runtime/state/widgets.py
Expand Up @@ -148,6 +148,26 @@
For both paths a widget return value is provided, allowing the widgets
to be used in a non-streamlit setting.
"""
# Some fields are common to most widgets and not part of the widget identity,
# so they should not be set before registration and we can check them here.
# They don't exist on every widget, so we ignore fields that don't exist.
try:
assert not element_proto.disabled
except AttributeError:
Fixed Show fixed Hide fixed
pass
try:
assert not element_proto.label_visibility.value
except AttributeError:
Fixed Show fixed Hide fixed
pass
try:
assert not element_proto.value
except AttributeError:
Fixed Show fixed Hide fixed
pass
try:
assert not element_proto.set_value
except AttributeError:
Fixed Show fixed Hide fixed
pass

widget_id = compute_widget_id(element_type, element_proto, user_key)
element_proto.id = widget_id

Expand Down
18 changes: 18 additions & 0 deletions lib/tests/streamlit/elements/slider_test.py
Expand Up @@ -25,6 +25,7 @@
from streamlit.errors import StreamlitAPIException
from streamlit.js_number import JSNumber
from streamlit.proto.LabelVisibilityMessage_pb2 import LabelVisibilityMessage
from streamlit.testing.script_interactions import InteractiveScriptTests
from tests.delta_generator_test_case import DeltaGeneratorTestCase


Expand Down Expand Up @@ -300,3 +301,20 @@ def test_label_visibility_wrong_value(self):
"Unsupported label_visibility option 'wrong_value'. Valid values are "
"'visible', 'hidden' or 'collapsed'.",
)


class SliderInteractiveTest(InteractiveScriptTests):
def test_id_stability(self):
script = self.script_from_string(
"""
import streamlit as st

st.slider("slider", key="slider")
"""
)
sr = script.run()
s1 = sr.slider[0]
sr2 = s1.set_value(5).run()
s2 = sr2.slider[0]

assert s1.id == s2.id