Conversation
The value of ui.refresh is typically a string, but it was documented to be an int. The value was weird in that it started as an int of 0, then changed to '<interval> (number of refreshes)`. Rather than keep this initialization behavior, this change updates ui.refresh's initial value to be an empty string. This is a breaking change, but at least they are both false-y. This change also updates typing and docs to say the value is a str.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the mo.ui.refresh UI element to reflect its actual runtime behavior: the element’s value is treated as a string (not an int), and the initial value is changed from 0 to "" to match the frontend’s emitted values and the updated documentation.
Changes:
- Change
refreshtyping fromUIElement[int, int]toUIElement[str, str]and update_convert_value/on_changetyping accordingly. - Update
refresh.valuedocumentation to describe the"<interval> (<count>)"string format. - Change the element’s
initial_valuefrom0to"".
Comments suppressed due to low confidence (1)
marimo/_plugins/ui/_impl/refresh.py:93
- The logic uses truthiness checks on
default_interval(if default_interval ...,[default_interval] if default_interval else [], and the membership check). This means falsy-but-valid values (e.g.0,0.0, or even invalid falsy values like[]) bypass validation and change behavior (e.g.default_interval=0won’t be included in options or checked for membership). Consider switching these conditions to explicitdefault_interval is not None(and, if desired, separately reject empty-string/zero values).
if default_interval and not isinstance(
default_interval, (int, float, str)
):
raise ValueError(
"Invalid type: `default_interval` must be "
+ f"an int, float or str, not {type(default_interval)}"
)
# If no options are provided and default_interval is provided,
if options is None:
resolved_options = [default_interval] if default_interval else []
else:
resolved_options = options
if not isinstance(resolved_options, list):
raise ValueError(
f"Invalid type: `options` must be a list, not {type(resolved_options)}"
)
# If has options and default_interval is provided,
# check that default_interval is in options.
if (
default_interval
and len(resolved_options) > 0
and (default_interval not in resolved_options)
):
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The value of ui.refresh is typically a string, but it was documented to be an int.
The value was weird in that it started as an int of 0, then changed to ' (number of refreshes)`.
Rather than keep this initialization behavior, this change updates ui.refresh's initial value to be an empty string. This is a breaking change, but at least they are both false-y.
This change also updates typing and docs to say the value is a str.
Fixes #9213