Metric supports numerics like decimal.#12377
Conversation
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) ✅ license/snyk check is complete. No issues have been found. (View Details) |
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
Co-authored-by: graphite-app[bot] <96075541+graphite-app[bot]@users.noreply.github.com>
lukasmasuch
added
security-assessment-completed
change:feature
lukasmasuch
added
status:needs-product-approval
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for numeric types like decimal.Decimal to the Streamlit metric element by expanding the accepted value types beyond the previously supported int, float, and str types.
Key Changes
- Extracts numeric-to-string conversion logic into a reusable
from_numberutility function instring_util.py - Updates the metric element to use the new utility function and accept
numbers.Numbertypes - Updates error messages to reflect the expanded type support
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
lib/streamlit/string_util.py |
Adds new from_number function and StringCastableNumber type alias to handle numeric-to-string conversion |
lib/streamlit/elements/metric.py |
Refactors _parse_value to use the new utility function and updates type annotations |
lib/tests/streamlit/string_util_test.py |
Adds unit tests for the new from_number function including decimal support |
lib/tests/streamlit/elements/metric_test.py |
Updates error message test to reflect new supported types |
|
Hey @aebrahim, thanks for the contribution! Does this PR only add support for decimal in |
|
That makes a lot of sense! I only added it to st.metric, but by doing it in a util, I thought that would make it easy to do it for any other place that needs it in the exact same way. I thought this was the best way to incorporate your feedback and still keep the pull request small. It would be very easy to move all calls to this util in every display element that needs to be changed in subsequent PR's. |
|
Got it. Would you be willing to work on supporting this for other elements as well, or not at the moment? What I want to avoid is that 1) we'll end up with partial support for this just on |
|
OK just talked to our engineers and we actually do already support decimal for The only other element where this might make sense is |
jrieke
added
status:product-approved
|
Sounds good, and I definitely don't mind doing a follow-up pull request to add it for any other types you need! |
|
just bumping to see if there are any other steps I need to take! |
|
@jrieke - anything else needed to get this merged? |
|
Bump! |
|
bump |
1 similar comment
|
bump |
|
bump! |
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: graphite-app[bot] <96075541+graphite-app[bot]@users.noreply.github.com>
lukasmasuch
left a comment
lukasmasuch
left a comment
There was a problem hiding this comment.
LGTM 👍 I added a few tweaks.
|
Thank you so much for helping with this! Best data dashboard there is :) |
Describe your changes
Add support for numeric types like
decimal.DecimalGitHub Issue Link (if applicable)
decimal.Decimalinst.metric#12308Testing Plan
Contribution License Agreement
By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.