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

Revised progress bar ETA #4054

Closed
willmcgugan opened this issue Jan 20, 2024 · 3 comments · Fixed by #4271
Closed

Revised progress bar ETA #4054

willmcgugan opened this issue Jan 20, 2024 · 3 comments · Fixed by #4271
Assignees
Labels
enhancement New feature or request Task

Comments

@willmcgugan
Copy link
Collaborator

willmcgugan commented Jan 20, 2024

The ProgressBar "ETA" is quite naive. I think it assumes a constant speed, which is rarely the case for things you would want a progress bar for.

We need a smarter way of calculating the ETA which emphasizes more recent data points. There are a few ways of doing this. You could see the Rich progress bars for an examples, but there may also be better algorithms. Suggesting asking Google or ChatGPT.

The implementation could also use another look. It adds a Horizontal container, where the widget itself could have a horizontal layout.

1d

@davep davep added enhancement New feature or request Task labels Jan 23, 2024
@davep davep self-assigned this Jan 31, 2024
davep added a commit to davep/textual that referenced this issue Jan 31, 2024
@davep
Copy link
Contributor

davep commented Feb 1, 2024

While making a start on this I ran into some confusing results and ended up uncovering #4096; so I think this will result in a revamp of how ProgressBar does the calculations even in the most simplistic of situations.

Some form of "please reset this ProgressBar for reuse" method might also be a good idea.

@davep
Copy link
Contributor

davep commented Feb 5, 2024

WiP on this has addressed #4096, but also uncovered a further issue where the ETAStatus appeared to be updating when it wasn't necessary; this has now been identified and fixed too.

An updated method of calculating the ETA is now in place and seems to produce reasonable results; for constant-time progress the timer looks spot on; and for accelerating, decelerating and reasonably random progress the results look good too, and generally more/less optimistic/pessimistic where appropriate.

Work on this is having further knock-on effects on how the documentation screen shots are generated, and also their use in snapshot tests; this will likely need some more work on how the docs for ProgressBar are handled, and might require removal of that code from snapshot tests. If the latter happens we'll need to do a bit of work on producing unit tests for the ETA calculation (which might require isolating the ETA code somewhat).

A question has also been raised about the newer approach taken to slide a window over the updates to perform the calculation.

+1d

@davep davep linked a pull request Mar 11, 2024 that will close this issue
@davep davep assigned willmcgugan and unassigned davep Mar 11, 2024
Copy link

Don't forget to star the repository!

Follow @textualizeio for Textual updates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Task
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants