Skip to content

Conversation

@mikeprosserni
Copy link
Collaborator

@mikeprosserni mikeprosserni commented Jun 16, 2025

What does this Pull Request accomplish?

Users want to be able to specify a default value for get_value() to return if the value has not been set yet.

We can implement this by simply catching the NOT_FOUND exception and returning the default, if there is one.

Why should this Pull Request be merged?

Implements AB#3161227

What testing has been done?

Manual testing:

with col3:
    st.markdown("**Default Values**")
    st.write(panel.get_value("missing_string", "0"))
    st.write(panel.get_value("missing_int", 0))
    st.write(panel.get_value("missing_float", 0.0))
    st.write(panel.get_value("missing_bool", False))
    st.write(panel.get_value("missing_list", [0.0]))

image

Also, new autotests:
test___set_value___get_value_ignores_default
test___get_value_returns_default_when_value_not_set
test___panel___get_unset_value_with_no_default___raises_exception

@github-actions
Copy link
Contributor

github-actions bot commented Jun 16, 2025

Test Results

 10 files  ± 0   10 suites  ±0   15s ⏱️ -7s
 80 tests + 2   80 ✅ + 2  0 💤 ±0  0 ❌ ±0 
770 runs  +20  770 ✅ +20  0 💤 ±0  0 ❌ ±0 

Results for commit 8b42206. ± Comparison against base commit 96498f7.

This pull request removes 1 and adds 3 tests. Note that renamed tests count towards both.
tests.unit.test_streamlit_panel ‑ test___panel___get_unset_value___raises_exception
tests.unit.test_streamlit_panel ‑ test___get_value_returns_default_when_value_not_set
tests.unit.test_streamlit_panel ‑ test___panel___get_unset_value_with_no_default___raises_exception
tests.unit.test_streamlit_panel ‑ test___set_value___get_value_ignores_default

♻️ This comment has been updated with latest results.

@mikeprosserni mikeprosserni marked this pull request as ready for review June 16, 2025 17:50
@mikeprosserni mikeprosserni requested a review from csjall as a code owner June 16, 2025 17:50
@mikeprosserni mikeprosserni merged commit 3ec3558 into main Jun 16, 2025
14 checks passed
@mikeprosserni mikeprosserni deleted the users/mprosser/techdebt-3161227-default-value branch June 16, 2025 19:48

def get_value(self, value_id: str) -> object:
"""Get the value for a control on the panel.
def get_value(self, value_id: str, default_value: object = None) -> object:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want the default value to affect type inference?

_T = TypeVar("_T")

@overload
def get_value(self, value_id: str) -> object: ...

@overload
def get_value(self, value_id: str, default_value: _T | None = None) -> _T: ...

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Successfully merging this pull request may close these issues.

4 participants