-
Notifications
You must be signed in to change notification settings - Fork 839
Show duration instead of rate for slow progress #7209
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
mscolnick
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice, thank you for this change! its much more readable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances the progress bar display by showing duration per iteration (e.g., "2 min, 2s per iter") instead of rate (e.g., "0.01 iter/s") when progress is slow (rate < 1 iter/s). This improves readability and prevents confusion from rounded rates that appear identical.
Key changes:
- Modified rate calculation to preserve precision for rates below 1 iter/s
- Updated frontend to display time-per-iteration format when rate < 1
- Adjusted
prettyTimefunction to omit decimal places when seconds >= 10
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
marimo/_plugins/stateless/status/_progress.py |
Modified _calculate_rate() to preserve full precision for rates < 1, updated docstring |
frontend/src/plugins/layout/ProgressPlugin.tsx |
Added conditional logic to display duration per iteration when rate < 1, modified prettyTime() to adjust decimal precision |
tests/_plugins/stateless/status/test_progress.py |
Added test_update_progress_slowly() to verify slow progress behavior with 12s sleep |
examples/outputs/progress_bar.py |
Added new example cell demonstrating slow progress with 12s delays |
marimo/_smoke_tests/async_iterator.py |
Added test_progress_slow_async() to test slow progress with varying durations |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…and split each case into its own test so all failures are reported.
|
Thank you both for the suggestions. I’ve mocked |
akshayka
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
📝 Summary
When using the progress bar the duration (e.g.
2 min, 2s per iter) of each loop will be shown instead of the rate (e.g.0.01 iter/s) when progress is slow.🔍 Description of Changes
When progress is slow the current implementation of the progress bar is not very helpful as it is harder to interpret
0.01 iter/sthan1 min 40s per iter. When the rate is very low the rounding also makes100sappear the same as100m(both0.01 iter/s). This commit detects when the rate drops below 1 iteration per second and switches the display to show time per iteration instead.Comparison Table
This uses the same humanizer that is used for the ETA, so the styles match. There is one small change to the humanizer: if the seconds are greater than 10, no decimal places are shown on the smaller unit, to avoid outputs like
10m, 2.04s.📋 Checklist