From cb6b1fba25db76564ee273aa675591b138200118 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Thu, 7 Mar 2024 16:33:27 +0000 Subject: [PATCH 01/15] improved eta --- src/textual/eta.py | 96 +++++++++++++++++++ src/textual/widgets/_progress_bar.py | 132 +++++++++------------------ 2 files changed, 139 insertions(+), 89 deletions(-) create mode 100644 src/textual/eta.py diff --git a/src/textual/eta.py b/src/textual/eta.py new file mode 100644 index 0000000000..334f690817 --- /dev/null +++ b/src/textual/eta.py @@ -0,0 +1,96 @@ +import bisect +from operator import itemgetter +from time import monotonic + + +class ETA: + """Calculate speed and estimate time to arrival.""" + + def __init__(self, estimation_period: float = 30) -> None: + """Create an ETA. + + Args: + estimation_period: Period in seconds, used to calculate speed. Defaults to 30. + """ + self.estimation_period = estimation_period + self._start_time = monotonic() + self._samples: list[tuple[float, float]] = [(0.0, 0.0)] + + def reset(self) -> None: + """Start ETA calculations from current time.""" + del self._samples[:] + self._start_time = monotonic() + + @property + def _current_time(self) -> float: + return monotonic() - self._start_time + + def add_sample(self, progress: float) -> None: + """Add a new sample. + + Args: + progress: Progress ratio (0 is start, 1 is complete). + """ + if self._samples and self._samples[-1][1] > progress: + # If progress goes backwards, we need to reset calculations + self.reset() + return + current_time = self._current_time + self._samples.append((current_time, progress)) + if not (len(self._samples) % 100): + # Prune periodically so we don't accumulate vast amounts of samples + self._prune() + + def _prune(self) -> None: + """Prune old samples.""" + if len(self._samples) <= 10: + # Keep at least 10 samples + return + prune_time = self._samples[-1][0] - self.estimation_period + index = bisect.bisect_left(self._samples, prune_time, key=itemgetter(0)) + del self._samples[:index] + + def _get_progress_at(self, time: float) -> tuple[float, float]: + """Get the progress at a specific time.""" + index = bisect.bisect_left(self._samples, time, key=itemgetter(0)) + if index >= len(self._samples): + return self._samples[-1] + if index == 0: + return self._samples[0] + time1, progress1 = self._samples[index] + time2, progress2 = self._samples[index + 1] + factor = (time - time1) / (time2 - time1) + intermediate_progress = progress1 + (progress2 - progress1) * factor + return time, intermediate_progress + + @property + def speed(self) -> float | None: + """The current speed, or `None` if it couldn't be calculated.""" + + if len(self._samples) <= 2: + # Need at less 2 samples to calculate speed + return None + + recent_sample_time, progress2 = self._samples[-1] + progress_start_time, progress1 = self._get_progress_at( + recent_sample_time - self.estimation_period + ) + time_delta = recent_sample_time - progress_start_time + distance = progress2 - progress1 + speed = distance / time_delta + return speed + + @property + def eta(self) -> float | None: + """Estimated seconds until completion, or `None` if no estimate can be made.""" + current_time = self._current_time + if not self._samples: + return None + speed = self.speed + if not speed: + return None + recent_time, recent_progress = self._samples[-1] + time_since_sample = current_time - recent_time + remaining = 1.0 - (recent_progress + speed * time_since_sample) + eta = max(0, remaining / speed) + return eta diff --git a/src/textual/widgets/_progress_bar.py b/src/textual/widgets/_progress_bar.py index fea9c2fba2..4a7ddd7916 100644 --- a/src/textual/widgets/_progress_bar.py +++ b/src/textual/widgets/_progress_bar.py @@ -4,16 +4,16 @@ from math import ceil from time import monotonic -from typing import Callable, Optional +from typing import Optional from rich.style import Style from .._types import UnusedParameter from ..app import ComposeResult, RenderResult +from ..eta import ETA from ..geometry import clamp from ..reactive import reactive from ..renderables.bar import Bar as BarRenderable -from ..timer import Timer from ..widget import Widget from ..widgets import Label @@ -80,7 +80,7 @@ def watch__percentage(self, percentage: float | None) -> None: if percentage is not None: self.auto_refresh = None else: - self.auto_refresh = 1 / 15 + self.auto_refresh = 1 / 5 def render(self) -> RenderResult: """Render the bar with the correct portion filled.""" @@ -105,6 +105,8 @@ def render_indeterminate(self) -> RenderResult: # Width used to enable the visual effect of the bar going into the corners. total_imaginary_width = width + highlighted_bar_width + start: float + end: float if self.app.animation_level == "none": start = 0 end = width @@ -188,15 +190,7 @@ class ETAStatus(Label): content-align-horizontal: right; } """ - - _label_text: reactive[str] = reactive("", repaint=False) - """This is used as an auxiliary reactive to only refresh the label when needed.""" - _percentage: reactive[float | None] = reactive[Optional[float]](None) - """The percentage of progress that has been completed.""" - _refresh_timer: Timer | None - """Timer to update ETA status even when progress stalls.""" - _start_time: float | None - """The time when the widget started tracking progress.""" + eta: reactive[float | None] = reactive[Optional[float]](None) def __init__( self, @@ -205,62 +199,24 @@ def __init__( classes: str | None = None, disabled: bool = False, ): - super().__init__(name=name, id=id, classes=classes, disabled=disabled) - self._percentage = None - self._label_text = "--:--:--" - self._start_time = None - self._refresh_timer = None - - def on_mount(self) -> None: - """Periodically refresh the countdown so that the ETA is always up to date.""" - self._refresh_timer = self.set_interval(1 / 2, self.update_eta, pause=True) + super().__init__( + "--:--:--", name=name, id=id, classes=classes, disabled=disabled + ) - def watch__percentage(self, percentage: float | None) -> None: - if percentage is None: - self._label_text = "--:--:--" - else: - if self._refresh_timer is not None: - self._refresh_timer.reset() - self.update_eta() - - def update_eta(self) -> None: - """Update the ETA display.""" - percentage = self._percentage - delta = self._get_elapsed_time() - # We display --:--:-- if we haven't started, if we are done, - # or if we don't know when we started keeping track of time. - if not percentage or percentage >= 1 or not delta: - self._label_text = "--:--:--" - # If we are done, we can delete the timer that periodically refreshes - # the countdown display. - if percentage is not None and percentage >= 1: - self.auto_refresh = None - # Render a countdown timer with hh:mm:ss, unless it's a LONG time. + def render(self) -> RenderResult: + """Render the ETA display.""" + eta = self.eta + if eta is None: + return "--:--:--" else: - left = ceil((delta / percentage) * (1 - percentage)) - minutes, seconds = divmod(left, 60) + minutes, seconds = divmod(ceil(eta), 60) hours, minutes = divmod(minutes, 60) if hours > 999999: - self._label_text = "+999999h" + return "+999999h" elif hours > 99: - self._label_text = f"{hours}h" + return f"{hours}h" else: - self._label_text = f"{hours:02}:{minutes:02}:{seconds:02}" - - def _get_elapsed_time(self) -> float: - """Get time to estimate time to progress completion. - - Returns: - The time elapsed since the bar started being animated. - """ - if self._start_time is None: - self._start_time = monotonic() - return 0 - return monotonic() - self._start_time - - def watch__label_text(self, label_text: str) -> None: - """If the ETA label changed, update the renderable (which also refreshes).""" - self.update(label_text) + return f"{hours:02}:{minutes:02}:{seconds:02}" class ProgressBar(Widget, can_focus=False): @@ -296,6 +252,7 @@ class ProgressBar(Widget, can_focus=False): print(progress_bar.percentage) # 0.5 ``` """ + _display_eta: reactive[float | None] = reactive[Optional[float]](None) def __init__( self, @@ -334,39 +291,28 @@ def key_space(self): disabled: Whether the widget is disabled or not. """ super().__init__(name=name, id=id, classes=classes, disabled=disabled) + self.total = total self.show_bar = show_bar self.show_percentage = show_percentage self.show_eta = show_eta + self._eta = ETA() - self.total = total - - def compose(self) -> ComposeResult: - # We create a closure so that we can determine what are the sub-widgets - # that are present and, therefore, will need to be notified about changes - # to the percentage. - def update_percentage( - widget: Bar | PercentageStatus | ETAStatus, - ) -> Callable[[float | None], None]: - """Closure to allow updating the percentage of a given widget.""" - - def updater(percentage: float | None) -> None: - """Update the percentage reactive of the enclosed widget.""" - widget._percentage = percentage + def on_mount(self) -> None: + def refresh_eta() -> None: + """Refresh eta display.""" + self._display_eta = self._eta.eta - return updater + self.set_interval(1 / 2, refresh_eta) + def compose(self) -> ComposeResult: if self.show_bar: - bar = Bar(id="bar") - self.watch(self, "percentage", update_percentage(bar)) - yield bar + yield Bar(id="bar").data_bind(_percentage=ProgressBar.percentage) if self.show_percentage: - percentage_status = PercentageStatus(id="percentage") - self.watch(self, "percentage", update_percentage(percentage_status)) - yield percentage_status + PercentageStatus(id="percentage").data_bind( + _percentage=ProgressBar.percentage + ) if self.show_eta: - eta_status = ETAStatus(id="eta") - self.watch(self, "percentage", update_percentage(eta_status)) - yield eta_status + yield ETAStatus(id="eta").data_bind(eta=ProgressBar._display_eta) def _validate_progress(self, progress: float) -> float: """Clamp the progress between 0 and the maximum total.""" @@ -406,7 +352,7 @@ def advance(self, advance: float = 1) -> None: Args: advance: Number of steps to advance progress by. """ - self.progress += advance + self.update(advance=advance) def update( self, @@ -431,8 +377,16 @@ def update( advance: Advance the progress by this number of steps. """ if not isinstance(total, UnusedParameter): + if total != self.total: + self._eta.reset() self.total = total - if not isinstance(progress, UnusedParameter): + + elif not isinstance(progress, UnusedParameter): self.progress = progress - if not isinstance(advance, UnusedParameter): + if self.progress is not None and self.total is not None: + self._eta.add_sample(self.progress / self.total) + + elif not isinstance(advance, UnusedParameter): self.progress += advance + if self.progress is not None and self.total is not None: + self._eta.add_sample(self.progress / self.total) From 670ea277b3a0fcdd23eda00d411130d1ffd76df8 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Thu, 7 Mar 2024 16:43:18 +0000 Subject: [PATCH 02/15] simplify --- src/textual/eta.py | 6 ++++-- src/textual/widgets/_progress_bar.py | 21 ++++----------------- 2 files changed, 8 insertions(+), 19 deletions(-) diff --git a/src/textual/eta.py b/src/textual/eta.py index 334f690817..5f22154212 100644 --- a/src/textual/eta.py +++ b/src/textual/eta.py @@ -1,4 +1,5 @@ import bisect +from math import ceil from operator import itemgetter from time import monotonic @@ -23,6 +24,7 @@ def reset(self) -> None: @property def _current_time(self) -> float: + """The time since the ETA was started.""" return monotonic() - self._start_time def add_sample(self, progress: float) -> None: @@ -81,7 +83,7 @@ def speed(self) -> float | None: return speed @property - def eta(self) -> float | None: + def eta(self) -> int | None: """Estimated seconds until completion, or `None` if no estimate can be made.""" current_time = self._current_time if not self._samples: @@ -93,4 +95,4 @@ def eta(self) -> float | None: time_since_sample = current_time - recent_time remaining = 1.0 - (recent_progress + speed * time_since_sample) eta = max(0, remaining / speed) - return eta + return ceil(eta) diff --git a/src/textual/widgets/_progress_bar.py b/src/textual/widgets/_progress_bar.py index 4a7ddd7916..de2c00f971 100644 --- a/src/textual/widgets/_progress_bar.py +++ b/src/textual/widgets/_progress_bar.py @@ -192,17 +192,6 @@ class ETAStatus(Label): """ eta: reactive[float | None] = reactive[Optional[float]](None) - def __init__( - self, - name: str | None = None, - id: str | None = None, - classes: str | None = None, - disabled: bool = False, - ): - super().__init__( - "--:--:--", name=name, id=id, classes=classes, disabled=disabled - ) - def render(self) -> RenderResult: """Render the ETA display.""" eta = self.eta @@ -252,7 +241,7 @@ class ProgressBar(Widget, can_focus=False): print(progress_bar.percentage) # 0.5 ``` """ - _display_eta: reactive[float | None] = reactive[Optional[float]](None) + _display_eta: reactive[int | None] = reactive[Optional[int]](None) def __init__( self, @@ -298,11 +287,7 @@ def key_space(self): self._eta = ETA() def on_mount(self) -> None: - def refresh_eta() -> None: - """Refresh eta display.""" - self._display_eta = self._eta.eta - - self.set_interval(1 / 2, refresh_eta) + self.set_interval(0.5, self.update) def compose(self) -> ComposeResult: if self.show_bar: @@ -390,3 +375,5 @@ def update( self.progress += advance if self.progress is not None and self.total is not None: self._eta.add_sample(self.progress / self.total) + + self._display_eta = self._eta.eta From 372d8187e8521778b393d2980600802ac7fb9333 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Thu, 7 Mar 2024 16:55:13 +0000 Subject: [PATCH 03/15] simplify --- src/textual/widgets/_progress_bar.py | 28 ++++------------------------ 1 file changed, 4 insertions(+), 24 deletions(-) diff --git a/src/textual/widgets/_progress_bar.py b/src/textual/widgets/_progress_bar.py index de2c00f971..505542b1a6 100644 --- a/src/textual/widgets/_progress_bar.py +++ b/src/textual/widgets/_progress_bar.py @@ -153,32 +153,12 @@ class PercentageStatus(Label): } """ - _label_text: reactive[str] = reactive("", repaint=False) - """This is used as an auxiliary reactive to only refresh the label when needed.""" _percentage: reactive[float | None] = reactive[Optional[float]](None) """The percentage of progress that has been completed.""" - def __init__( - self, - name: str | None = None, - id: str | None = None, - classes: str | None = None, - disabled: bool = False, - ): - super().__init__(name=name, id=id, classes=classes, disabled=disabled) - self._percentage = None - self._label_text = "--%" - - def watch__percentage(self, percentage: float | None) -> None: - """Manage the text that shows the percentage of progress.""" - if percentage is None: - self._label_text = "--%" - else: - self._label_text = f"{int(100 * percentage)}%" - - def watch__label_text(self, label_text: str) -> None: - """If the label text changed, update the renderable (which also refreshes).""" - self.update(label_text) + def render(self) -> RenderResult: + percentage = self._percentage + return "--%" if percentage is None else f"{int(100 * percentage)}%" class ETAStatus(Label): @@ -293,7 +273,7 @@ def compose(self) -> ComposeResult: if self.show_bar: yield Bar(id="bar").data_bind(_percentage=ProgressBar.percentage) if self.show_percentage: - PercentageStatus(id="percentage").data_bind( + yield PercentageStatus(id="percentage").data_bind( _percentage=ProgressBar.percentage ) if self.show_eta: From ea8e75661aa4d932cc8a74211416fa326501fd18 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Sat, 9 Mar 2024 12:44:22 +0000 Subject: [PATCH 04/15] snapshot --- .../widgets/progress_bar_isolated_.py | 15 +- docs/examples/widgets/progress_bar_styled_.py | 14 +- src/textual/eta.py | 26 +- src/textual/widgets/_progress_bar.py | 35 +-- .../__snapshots__/test_snapshots.ambr | 234 +++++++++--------- .../snapshot_tests/snapshot_apps/recompose.py | 2 +- .../snapshot_tests/snapshot_apps/tooltips.py | 2 +- tests/test_progress_bar.py | 1 - 8 files changed, 175 insertions(+), 154 deletions(-) diff --git a/docs/examples/widgets/progress_bar_isolated_.py b/docs/examples/widgets/progress_bar_isolated_.py index 79907562cf..f6c2d4eaac 100644 --- a/docs/examples/widgets/progress_bar_isolated_.py +++ b/docs/examples/widgets/progress_bar_isolated_.py @@ -11,13 +11,14 @@ class IndeterminateProgressBar(App[None]): """Timer to simulate progress happening.""" def compose(self) -> ComposeResult: + self.time = 0 with Center(): with Middle(): - yield ProgressBar() + yield ProgressBar(_get_time=lambda: self.time) yield Footer() def on_mount(self) -> None: - """Set up a timer to simulate progess happening.""" + """Set up a timer to simulate progress happening.""" self.progress_timer = self.set_interval(1 / 10, self.make_progress, pause=True) def make_progress(self) -> None: @@ -31,14 +32,18 @@ def action_start(self) -> None: def key_f(self) -> None: # Freeze time for the indeterminate progress bar. - self.query_one(ProgressBar).query_one("#bar")._get_elapsed_time = lambda: 5 + self.time = 5 + self.refresh() def key_t(self) -> None: # Freeze time to show always the same ETA. - self.query_one(ProgressBar).query_one("#eta")._get_elapsed_time = lambda: 3.9 - self.query_one(ProgressBar).update(total=100, progress=39) + self.time = 0 + self.query_one(ProgressBar).update(total=100, progress=0) + self.time = 3.9 + self.query_one(ProgressBar).update(progress=39) def key_u(self) -> None: + self.refresh() self.query_one(ProgressBar).update(total=100, progress=100) diff --git a/docs/examples/widgets/progress_bar_styled_.py b/docs/examples/widgets/progress_bar_styled_.py index 8428f359a1..9da9a75f21 100644 --- a/docs/examples/widgets/progress_bar_styled_.py +++ b/docs/examples/widgets/progress_bar_styled_.py @@ -12,13 +12,14 @@ class StyledProgressBar(App[None]): """Timer to simulate progress happening.""" def compose(self) -> ComposeResult: + self.time = 0 with Center(): with Middle(): - yield ProgressBar() + yield ProgressBar(_get_time=lambda: self.time) yield Footer() def on_mount(self) -> None: - """Set up a timer to simulate progess happening.""" + """Set up a timer to simulate progress happening.""" self.progress_timer = self.set_interval(1 / 10, self.make_progress, pause=True) def make_progress(self) -> None: @@ -32,12 +33,15 @@ def action_start(self) -> None: def key_f(self) -> None: # Freeze time for the indeterminate progress bar. - self.query_one(ProgressBar).query_one("#bar")._get_elapsed_time = lambda: 5 + self.time = 5 + self.refresh() def key_t(self) -> None: # Freeze time to show always the same ETA. - self.query_one(ProgressBar).query_one("#eta")._get_elapsed_time = lambda: 3.9 - self.query_one(ProgressBar).update(total=100, progress=39) + self.time = 0 + self.query_one(ProgressBar).update(total=100, progress=0) + self.time = 3.9 + self.query_one(ProgressBar).update(progress=39) def key_u(self) -> None: self.query_one(ProgressBar).update(total=100, progress=100) diff --git a/src/textual/eta.py b/src/textual/eta.py index 5f22154212..f432d17513 100644 --- a/src/textual/eta.py +++ b/src/textual/eta.py @@ -2,30 +2,42 @@ from math import ceil from operator import itemgetter from time import monotonic +from typing import Callable +import rich.repr + +@rich.repr.auto(angular=True) class ETA: """Calculate speed and estimate time to arrival.""" - def __init__(self, estimation_period: float = 30) -> None: + def __init__( + self, estimation_period: float = 30, _get_time: Callable[[], float] = monotonic + ) -> None: """Create an ETA. Args: estimation_period: Period in seconds, used to calculate speed. Defaults to 30. + _get_time: Optional replacement function to get current time. """ self.estimation_period = estimation_period - self._start_time = monotonic() + self._get_time = _get_time + self._start_time = _get_time() self._samples: list[tuple[float, float]] = [(0.0, 0.0)] + def __rich_repr__(self) -> rich.repr.Result: + yield "speed", self.speed + yield "eta", self.eta + def reset(self) -> None: """Start ETA calculations from current time.""" del self._samples[:] - self._start_time = monotonic() + self._start_time = self._get_time() @property def _current_time(self) -> float: """The time since the ETA was started.""" - return monotonic() - self._start_time + return self._get_time() - self._start_time def add_sample(self, progress: float) -> None: """Add a new sample. @@ -69,7 +81,7 @@ def _get_progress_at(self, time: float) -> tuple[float, float]: def speed(self) -> float | None: """The current speed, or `None` if it couldn't be calculated.""" - if len(self._samples) <= 2: + if len(self._samples) < 2: # Need at less 2 samples to calculate speed return None @@ -79,15 +91,13 @@ def speed(self) -> float | None: ) time_delta = recent_sample_time - progress_start_time distance = progress2 - progress1 - speed = distance / time_delta + speed = distance / time_delta if time_delta else 0 return speed @property def eta(self) -> int | None: """Estimated seconds until completion, or `None` if no estimate can be made.""" current_time = self._current_time - if not self._samples: - return None speed = self.speed if not speed: return None diff --git a/src/textual/widgets/_progress_bar.py b/src/textual/widgets/_progress_bar.py index 505542b1a6..d22a46f9eb 100644 --- a/src/textual/widgets/_progress_bar.py +++ b/src/textual/widgets/_progress_bar.py @@ -4,7 +4,7 @@ from math import ceil from time import monotonic -from typing import Optional +from typing import Callable, Optional from rich.style import Style @@ -69,8 +69,10 @@ def __init__( id: str | None = None, classes: str | None = None, disabled: bool = False, + _get_time: Callable[[], float] = monotonic, ): """Create a bar for a [`ProgressBar`][textual.widgets.ProgressBar].""" + self._get_time = _get_time super().__init__(name=name, id=id, classes=classes, disabled=disabled) self._start_time = None self._percentage = None @@ -138,9 +140,9 @@ def _get_elapsed_time(self) -> float: The time elapsed since the bar started being animated. """ if self._start_time is None: - self._start_time = monotonic() + self._start_time = self._get_time() return 0 - return monotonic() - self._start_time + return self._get_time() - self._start_time class PercentageStatus(Label): @@ -171,6 +173,7 @@ class ETAStatus(Label): } """ eta: reactive[float | None] = reactive[Optional[float]](None) + """Estimated number of seconds till completion.""" def render(self) -> RenderResult: """Render the ETA display.""" @@ -234,6 +237,7 @@ def __init__( id: str | None = None, classes: str | None = None, disabled: bool = False, + _get_time: Callable[[], float] = monotonic, ): """Create a Progress Bar widget. @@ -264,14 +268,18 @@ def key_space(self): self.show_bar = show_bar self.show_percentage = show_percentage self.show_eta = show_eta - self._eta = ETA() + self._get_time = _get_time + self._eta = ETA(_get_time=_get_time) def on_mount(self) -> None: + self.update() self.set_interval(0.5, self.update) def compose(self) -> ComposeResult: if self.show_bar: - yield Bar(id="bar").data_bind(_percentage=ProgressBar.percentage) + yield Bar(id="bar", _get_time=self._get_time).data_bind( + _percentage=ProgressBar.percentage + ) if self.show_percentage: yield PercentageStatus(id="percentage").data_bind( _percentage=ProgressBar.percentage @@ -291,17 +299,13 @@ def _validate_total(self, total: float | None) -> float | None: return total return max(0, total) - def _watch_total(self) -> None: - """Re-validate progress.""" - self.progress = self.progress - def _compute_percentage(self) -> float | None: """Keep the percentage of progress updated automatically. This will report a percentage of `1` if the total is zero. """ if self.total: - return self.progress / self.total + return min(1.0, self.progress / self.total) elif self.total == 0: return 1 return None @@ -346,14 +350,13 @@ def update( self._eta.reset() self.total = total - elif not isinstance(progress, UnusedParameter): + if not isinstance(progress, UnusedParameter): self.progress = progress - if self.progress is not None and self.total is not None: - self._eta.add_sample(self.progress / self.total) - elif not isinstance(advance, UnusedParameter): + if not isinstance(advance, UnusedParameter): self.progress += advance - if self.progress is not None and self.total is not None: - self._eta.add_sample(self.progress / self.total) + + if self.progress is not None and self.total is not None: + self._eta.add_sample(self.progress / self.total) self._display_eta = self._eta.eta diff --git a/tests/snapshot_tests/__snapshots__/test_snapshots.ambr b/tests/snapshot_tests/__snapshots__/test_snapshots.ambr index 04502cde5c..f278f078f9 100644 --- a/tests/snapshot_tests/__snapshots__/test_snapshots.ambr +++ b/tests/snapshot_tests/__snapshots__/test_snapshots.ambr @@ -29774,135 +29774,135 @@ font-weight: 700; } - .terminal-904522218-matrix { + .terminal-1786282230-matrix { font-family: Fira Code, monospace; font-size: 20px; line-height: 24.4px; font-variant-east-asian: full-width; } - .terminal-904522218-title { + .terminal-1786282230-title { font-size: 18px; font-weight: bold; font-family: arial; } - .terminal-904522218-r1 { fill: #ff0000 } - .terminal-904522218-r2 { fill: #c5c8c6 } - .terminal-904522218-r3 { fill: #e1e1e1;font-weight: bold } - .terminal-904522218-r4 { fill: #e1e1e1 } - .terminal-904522218-r5 { fill: #fea62b } - .terminal-904522218-r6 { fill: #323232 } + .terminal-1786282230-r1 { fill: #ff0000 } + .terminal-1786282230-r2 { fill: #c5c8c6 } + .terminal-1786282230-r3 { fill: #e1e1e1;font-weight: bold } + .terminal-1786282230-r4 { fill: #e1e1e1 } + .terminal-1786282230-r5 { fill: #fea62b } + .terminal-1786282230-r6 { fill: #323232 } - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - RecomposeApp + RecomposeApp - - - - ┌─────────┐┌─────────┐┌──────────┐┌─────────┐┌──────────┐┌─────────┐┌──────────┐ - │ ┓ ┏━┓││ ┓  ┓ ││ ┓ ╺━┓││ ┓ ╺━┓││ ┓ ╻ ╻││ ┓ ┏━╸││ ┓ ┏━╸│ - │ ┃ ┃ ┃││ ┃  ┃ ││ ┃ ┏━┛││ ┃  ━┫││ ┃ ┗━┫││ ┃ ┗━┓││ ┃ ┣━┓│ - │╺┻╸┗━┛││╺┻╸╺┻╸││╺┻╸┗━╸││╺┻╸╺━┛││╺┻╸  ╹││╺┻╸╺━┛││╺┻╸┗━┛│ - └─────────┘└─────────┘└──────────┘└─────────┘└──────────┘└─────────┘└──────────┘ - - - - - - - - ━━━━━━━━━━━━━━━━╺━━━━━━━━━━━━━━━50%--:--:-- - - - - - - - - - - + + + + ┌─────────┐┌─────────┐┌──────────┐┌─────────┐┌──────────┐┌─────────┐┌──────────┐ + │ ┓ ┏━┓││ ┓  ┓ ││ ┓ ╺━┓││ ┓ ╺━┓││ ┓ ╻ ╻││ ┓ ┏━╸││ ┓ ┏━╸│ + │ ┃ ┃ ┃││ ┃  ┃ ││ ┃ ┏━┛││ ┃  ━┫││ ┃ ┗━┫││ ┃ ┗━┓││ ┃ ┣━┓│ + │╺┻╸┗━┛││╺┻╸╺┻╸││╺┻╸┗━╸││╺┻╸╺━┛││╺┻╸  ╹││╺┻╸╺━┛││╺┻╸┗━┛│ + └─────────┘└─────────┘└──────────┘└─────────┘└──────────┘└─────────┘└──────────┘ + + + + + + + + ━━━━━━━━━━━━━━━━╺━━━━━━━━━━━━━━━50% + + + + + + + + + + @@ -40274,134 +40274,134 @@ font-weight: 700; } - .terminal-3455460968-matrix { + .terminal-3216424293-matrix { font-family: Fira Code, monospace; font-size: 20px; line-height: 24.4px; font-variant-east-asian: full-width; } - .terminal-3455460968-title { + .terminal-3216424293-title { font-size: 18px; font-weight: bold; font-family: arial; } - .terminal-3455460968-r1 { fill: #fea62b } - .terminal-3455460968-r2 { fill: #323232 } - .terminal-3455460968-r3 { fill: #c5c8c6 } - .terminal-3455460968-r4 { fill: #e1e1e1 } - .terminal-3455460968-r5 { fill: #e2e3e3 } + .terminal-3216424293-r1 { fill: #fea62b } + .terminal-3216424293-r2 { fill: #323232 } + .terminal-3216424293-r3 { fill: #c5c8c6 } + .terminal-3216424293-r4 { fill: #e1e1e1 } + .terminal-3216424293-r5 { fill: #e2e3e3 } - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - TooltipApp + TooltipApp - - - - ━━━╺━━━━━━━━━━━━━━━━━━━━━━━━━━━━10%--:--:-- - - Hello, Tooltip! - - - - - - - - - - - - - - - - - - - - + + + + ━━━╺━━━━━━━━━━━━━━━━━━━━━━━━━━━━10% + + Hello, Tooltip! + + + + + + + + + + + + + + + + + + + + diff --git a/tests/snapshot_tests/snapshot_apps/recompose.py b/tests/snapshot_tests/snapshot_apps/recompose.py index 8275289eeb..f0e5909f95 100644 --- a/tests/snapshot_tests/snapshot_apps/recompose.py +++ b/tests/snapshot_tests/snapshot_apps/recompose.py @@ -29,7 +29,7 @@ class Progress(Horizontal): progress = reactive(0, recompose=True) def compose(self) -> ComposeResult: - bar = ProgressBar(100) + bar = ProgressBar(100, show_eta=False) bar.progress = self.progress yield bar diff --git a/tests/snapshot_tests/snapshot_apps/tooltips.py b/tests/snapshot_tests/snapshot_apps/tooltips.py index bc55542d71..6c9ca8aea5 100644 --- a/tests/snapshot_tests/snapshot_apps/tooltips.py +++ b/tests/snapshot_tests/snapshot_apps/tooltips.py @@ -4,7 +4,7 @@ class TooltipApp(App[None]): def compose(self) -> ComposeResult: - progress_bar = ProgressBar(100) + progress_bar = ProgressBar(100, show_eta=False) progress_bar.advance(10) progress_bar.tooltip = "Hello, Tooltip!" yield progress_bar diff --git a/tests/test_progress_bar.py b/tests/test_progress_bar.py index bc7f799196..3cc3a42bfc 100644 --- a/tests/test_progress_bar.py +++ b/tests/test_progress_bar.py @@ -52,7 +52,6 @@ def test_progress_overflow(): assert pb.percentage == 1 pb.update(total=50) - assert pb.progress == 50 assert pb.percentage == 1 From ec552f0bd99c0e502dd0c6e4745d166af8b05739 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Sat, 9 Mar 2024 17:42:31 +0000 Subject: [PATCH 05/15] simplify --- src/textual/eta.py | 67 +++++++++++++++------------- src/textual/widgets/_progress_bar.py | 51 ++++++++++----------- 2 files changed, 60 insertions(+), 58 deletions(-) diff --git a/src/textual/eta.py b/src/textual/eta.py index f432d17513..713f482758 100644 --- a/src/textual/eta.py +++ b/src/textual/eta.py @@ -1,8 +1,8 @@ +from __future__ import annotations + import bisect -from math import ceil from operator import itemgetter from time import monotonic -from typing import Callable import rich.repr @@ -11,47 +11,50 @@ class ETA: """Calculate speed and estimate time to arrival.""" - def __init__( - self, estimation_period: float = 30, _get_time: Callable[[], float] = monotonic - ) -> None: + def __init__(self, estimation_period: float = 60) -> None: """Create an ETA. Args: estimation_period: Period in seconds, used to calculate speed. Defaults to 30. - _get_time: Optional replacement function to get current time. """ self.estimation_period = estimation_period - self._get_time = _get_time - self._start_time = _get_time() self._samples: list[tuple[float, float]] = [(0.0, 0.0)] + self._add_count = 0 def __rich_repr__(self) -> rich.repr.Result: yield "speed", self.speed - yield "eta", self.eta + yield "eta", self.get_eta(monotonic()) + + @property + def first_sample(self) -> tuple[float, float]: + """First sample, or `None` if no samples.""" + assert self._samples, "Assumes samples not empty" + return self._samples[0] + + @property + def last_sample(self) -> tuple[float, float]: + """Last sample, or `None` if no samples.""" + assert self._samples, "Assumes samples not empty" + return self._samples[-1] def reset(self) -> None: """Start ETA calculations from current time.""" del self._samples[:] - self._start_time = self._get_time() - @property - def _current_time(self) -> float: - """The time since the ETA was started.""" - return self._get_time() - self._start_time - - def add_sample(self, progress: float) -> None: + def add_sample(self, time: float, progress: float) -> None: """Add a new sample. Args: + time: Time when sample occurred. progress: Progress ratio (0 is start, 1 is complete). """ - if self._samples and self._samples[-1][1] > progress: + if self._samples and self.last_sample[1] > progress: # If progress goes backwards, we need to reset calculations self.reset() return - current_time = self._current_time - self._samples.append((current_time, progress)) - if not (len(self._samples) % 100): + self._samples.append((time, progress)) + self._add_count += 1 + if self._add_count % 100 == 0: # Prune periodically so we don't accumulate vast amounts of samples self._prune() @@ -68,9 +71,10 @@ def _get_progress_at(self, time: float) -> tuple[float, float]: """Get the progress at a specific time.""" index = bisect.bisect_left(self._samples, time, key=itemgetter(0)) if index >= len(self._samples): - return self._samples[-1] + return self.last_sample if index == 0: - return self._samples[0] + return self.first_sample + # Linearly interpolate progress between two samples time1, progress1 = self._samples[index] time2, progress2 = self._samples[index + 1] factor = (time - time1) / (time2 - time1) @@ -85,7 +89,7 @@ def speed(self) -> float | None: # Need at less 2 samples to calculate speed return None - recent_sample_time, progress2 = self._samples[-1] + recent_sample_time, progress2 = self.last_sample progress_start_time, progress1 = self._get_progress_at( recent_sample_time - self.estimation_period ) @@ -94,15 +98,18 @@ def speed(self) -> float | None: speed = distance / time_delta if time_delta else 0 return speed - @property - def eta(self) -> int | None: - """Estimated seconds until completion, or `None` if no estimate can be made.""" - current_time = self._current_time + def get_eta(self, time: float) -> int | None: + """Estimated seconds until completion, or `None` if no estimate can be made. + + Args: + time: Current time. + """ speed = self.speed if not speed: + # Not enough samples to guess return None - recent_time, recent_progress = self._samples[-1] - time_since_sample = current_time - recent_time + recent_time, recent_progress = self.last_sample + time_since_sample = time - recent_time remaining = 1.0 - (recent_progress + speed * time_since_sample) eta = max(0, remaining / speed) - return ceil(eta) + return round(eta) diff --git a/src/textual/widgets/_progress_bar.py b/src/textual/widgets/_progress_bar.py index d22a46f9eb..1358f84961 100644 --- a/src/textual/widgets/_progress_bar.py +++ b/src/textual/widgets/_progress_bar.py @@ -2,7 +2,6 @@ from __future__ import annotations -from math import ceil from time import monotonic from typing import Callable, Optional @@ -11,7 +10,6 @@ from .._types import UnusedParameter from ..app import ComposeResult, RenderResult from ..eta import ETA -from ..geometry import clamp from ..reactive import reactive from ..renderables.bar import Bar as BarRenderable from ..widget import Widget @@ -58,7 +56,7 @@ class Bar(Widget, can_focus=False): } """ - _percentage: reactive[float | None] = reactive[Optional[float]](None) + percentage: reactive[float | None] = reactive[float | None](None) """The percentage of progress that has been completed.""" _start_time: float | None """The time when the widget started tracking progress.""" @@ -75,27 +73,32 @@ def __init__( self._get_time = _get_time super().__init__(name=name, id=id, classes=classes, disabled=disabled) self._start_time = None - self._percentage = None + self.percentage = None - def watch__percentage(self, percentage: float | None) -> None: + def _validate_percentage(self, percentage: float | None) -> float | None: + """Avoid updating the bar, if the percentage increase is to small to render.""" + width = self.size.width * 2 + return None if percentage is None else int(percentage * width) / width + + def watch_percentage(self, percentage: float | None) -> None: """Manage the timer that enables the indeterminate bar animation.""" if percentage is not None: self.auto_refresh = None else: - self.auto_refresh = 1 / 5 + self.auto_refresh = 1 / 15 def render(self) -> RenderResult: """Render the bar with the correct portion filled.""" - if self._percentage is None: + if self.percentage is None: return self.render_indeterminate() else: bar_style = ( self.get_component_rich_style("bar--bar") - if self._percentage < 1 + if self.percentage < 1 else self.get_component_rich_style("bar--complete") ) return BarRenderable( - highlight_range=(0, self.size.width * self._percentage), + highlight_range=(0, self.size.width * self.percentage), highlight_style=Style.from_color(bar_style.color), background_style=Style.from_color(bar_style.bgcolor), ) @@ -155,12 +158,14 @@ class PercentageStatus(Label): } """ - _percentage: reactive[float | None] = reactive[Optional[float]](None) + percentage: reactive[int | None] = reactive[Optional[int]](None) """The percentage of progress that has been completed.""" + def _validate_percentage(self, percentage: float | None) -> int | None: + return None if percentage is None else round(percentage * 100) + def render(self) -> RenderResult: - percentage = self._percentage - return "--%" if percentage is None else f"{int(100 * percentage)}%" + return "--%" if self.percentage is None else f"{self.percentage}%" class ETAStatus(Label): @@ -181,7 +186,7 @@ def render(self) -> RenderResult: if eta is None: return "--:--:--" else: - minutes, seconds = divmod(ceil(eta), 60) + minutes, seconds = divmod(round(eta), 60) hours, minutes = divmod(minutes, 60) if hours > 999999: return "+999999h" @@ -269,7 +274,7 @@ def key_space(self): self.show_percentage = show_percentage self.show_eta = show_eta self._get_time = _get_time - self._eta = ETA(_get_time=_get_time) + self._eta = ETA() def on_mount(self) -> None: self.update() @@ -277,22 +282,12 @@ def on_mount(self) -> None: def compose(self) -> ComposeResult: if self.show_bar: - yield Bar(id="bar", _get_time=self._get_time).data_bind( - _percentage=ProgressBar.percentage - ) + yield Bar(id="bar").data_bind(ProgressBar.percentage) if self.show_percentage: - yield PercentageStatus(id="percentage").data_bind( - _percentage=ProgressBar.percentage - ) + yield PercentageStatus(id="percentage").data_bind(ProgressBar.percentage) if self.show_eta: yield ETAStatus(id="eta").data_bind(eta=ProgressBar._display_eta) - def _validate_progress(self, progress: float) -> float: - """Clamp the progress between 0 and the maximum total.""" - if self.total is not None: - return clamp(progress, 0, self.total) - return progress - def _validate_total(self, total: float | None) -> float | None: """Ensure the total is not negative.""" if total is None: @@ -357,6 +352,6 @@ def update( self.progress += advance if self.progress is not None and self.total is not None: - self._eta.add_sample(self.progress / self.total) + self._eta.add_sample(self._get_time(), self.progress / self.total) - self._display_eta = self._eta.eta + self._display_eta = self._eta.get_eta(self._get_time()) From 05ec4ab08aab935cab5b7f349d97d5eecf48fa13 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Sat, 9 Mar 2024 18:17:11 +0000 Subject: [PATCH 06/15] fix typing --- src/textual/widgets/_progress_bar.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/textual/widgets/_progress_bar.py b/src/textual/widgets/_progress_bar.py index 1358f84961..430cd9871d 100644 --- a/src/textual/widgets/_progress_bar.py +++ b/src/textual/widgets/_progress_bar.py @@ -56,7 +56,7 @@ class Bar(Widget, can_focus=False): } """ - percentage: reactive[float | None] = reactive[float | None](None) + percentage: reactive[float | None] = reactive[Optional[float]](None) """The percentage of progress that has been completed.""" _start_time: float | None """The time when the widget started tracking progress.""" From 5cff594512225ebb646c486d78f266a4d2a8fab6 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Sun, 10 Mar 2024 15:16:38 +0000 Subject: [PATCH 07/15] tests --- .../widgets/progress_bar_isolated_.py | 11 +- docs/examples/widgets/progress_bar_styled.py | 2 +- docs/examples/widgets/progress_bar_styled_.py | 13 +- src/textual/clock.py | 74 ++++++ src/textual/eta.py | 3 +- src/textual/widgets/_progress_bar.py | 59 ++--- .../__snapshots__/test_snapshots.ambr | 240 +++++++++--------- tests/test_progress_bar.py | 2 - 8 files changed, 234 insertions(+), 170 deletions(-) create mode 100644 src/textual/clock.py diff --git a/docs/examples/widgets/progress_bar_isolated_.py b/docs/examples/widgets/progress_bar_isolated_.py index f6c2d4eaac..c1df28d5db 100644 --- a/docs/examples/widgets/progress_bar_isolated_.py +++ b/docs/examples/widgets/progress_bar_isolated_.py @@ -1,4 +1,5 @@ from textual.app import App, ComposeResult +from textual.clock import MockClock from textual.containers import Center, Middle from textual.timer import Timer from textual.widgets import Footer, ProgressBar @@ -11,10 +12,10 @@ class IndeterminateProgressBar(App[None]): """Timer to simulate progress happening.""" def compose(self) -> ComposeResult: - self.time = 0 + self.clock = MockClock() with Center(): with Middle(): - yield ProgressBar(_get_time=lambda: self.time) + yield ProgressBar(clock=self.clock) yield Footer() def on_mount(self) -> None: @@ -32,14 +33,14 @@ def action_start(self) -> None: def key_f(self) -> None: # Freeze time for the indeterminate progress bar. - self.time = 5 + self.clock.set_time(5) self.refresh() def key_t(self) -> None: # Freeze time to show always the same ETA. - self.time = 0 + self.clock.set_time(0) self.query_one(ProgressBar).update(total=100, progress=0) - self.time = 3.9 + self.clock.set_time(3.9) self.query_one(ProgressBar).update(progress=39) def key_u(self) -> None: diff --git a/docs/examples/widgets/progress_bar_styled.py b/docs/examples/widgets/progress_bar_styled.py index 96c5005bab..f5b95ad055 100644 --- a/docs/examples/widgets/progress_bar_styled.py +++ b/docs/examples/widgets/progress_bar_styled.py @@ -18,7 +18,7 @@ def compose(self) -> ComposeResult: yield Footer() def on_mount(self) -> None: - """Set up a timer to simulate progess happening.""" + """Set up a timer to simulate progress happening.""" self.progress_timer = self.set_interval(1 / 10, self.make_progress, pause=True) def make_progress(self) -> None: diff --git a/docs/examples/widgets/progress_bar_styled_.py b/docs/examples/widgets/progress_bar_styled_.py index 9da9a75f21..b195327567 100644 --- a/docs/examples/widgets/progress_bar_styled_.py +++ b/docs/examples/widgets/progress_bar_styled_.py @@ -1,4 +1,5 @@ from textual.app import App, ComposeResult +from textual.clock import MockClock from textual.containers import Center, Middle from textual.timer import Timer from textual.widgets import Footer, ProgressBar @@ -12,10 +13,10 @@ class StyledProgressBar(App[None]): """Timer to simulate progress happening.""" def compose(self) -> ComposeResult: - self.time = 0 + self.clock = MockClock() with Center(): with Middle(): - yield ProgressBar(_get_time=lambda: self.time) + yield ProgressBar(clock=self.clock) yield Footer() def on_mount(self) -> None: @@ -30,17 +31,17 @@ def action_start(self) -> None: """Start the progress tracking.""" self.query_one(ProgressBar).update(total=100) self.progress_timer.resume() + self.query_one(ProgressBar).refresh() def key_f(self) -> None: # Freeze time for the indeterminate progress bar. - self.time = 5 - self.refresh() + self.clock.set_time(5.0) def key_t(self) -> None: # Freeze time to show always the same ETA. - self.time = 0 + self.clock.set_time(0) self.query_one(ProgressBar).update(total=100, progress=0) - self.time = 3.9 + self.clock.set_time(3.9) self.query_one(ProgressBar).update(progress=39) def key_u(self) -> None: diff --git a/src/textual/clock.py b/src/textual/clock.py new file mode 100644 index 0000000000..6df651bcc9 --- /dev/null +++ b/src/textual/clock.py @@ -0,0 +1,74 @@ +from __future__ import annotations + +from time import monotonic +from typing import Callable + +import rich.repr + + +@rich.repr.auto(angular=True) +class Clock: + """An object to get relative time. + + The `time` attribute of clock will return the time in seconds since the + Clock was created or reset. + + """ + + def __init__(self, *, get_time: Callable[[], float] = monotonic) -> None: + """Create a clock. + + Args: + get_time: A callable to get time in seconds. + start: Start the clock (time is 0 unless clock has been started). + """ + self._get_time = get_time + self._start_time = self._get_time() + + def __rich_repr__(self) -> rich.repr.Result: + yield self.time + + def clone(self) -> Clock: + """Clone the Clock with an independent time.""" + return Clock(get_time=self._get_time) + + def reset(self) -> None: + """Reset the clock.""" + self._start_time = self._get_time() + + @property + def time(self) -> float: + """Time since creation or reset.""" + return self._get_time() - self._start_time + + +class MockClock(Clock): + """A mock clock object where the time may be explicitly set.""" + + def __init__(self, time: float = 0.0) -> None: + """Construct a mock clock.""" + self._time = time + super().__init__(get_time=lambda: self._time) + + def clone(self) -> MockClock: + """Clone the mocked clock (clone will return the same time as original).""" + clock = MockClock(self._time) + clock._get_time = self._get_time + clock._time = self._time + return clock + + def reset(self) -> None: + """A null-op because it doesn't make sense to reset a mocked clock.""" + + def set_time(self, time: float) -> None: + """Set the time for the clock. + + Args: + time: Time to set. + """ + self._time = time + + @property + def time(self) -> float: + """Time since creation or reset.""" + return self._get_time() diff --git a/src/textual/eta.py b/src/textual/eta.py index 713f482758..4f5d8761b7 100644 --- a/src/textual/eta.py +++ b/src/textual/eta.py @@ -1,6 +1,7 @@ from __future__ import annotations import bisect +from math import ceil from operator import itemgetter from time import monotonic @@ -112,4 +113,4 @@ def get_eta(self, time: float) -> int | None: time_since_sample = time - recent_time remaining = 1.0 - (recent_progress + speed * time_since_sample) eta = max(0, remaining / speed) - return round(eta) + return ceil(eta) diff --git a/src/textual/widgets/_progress_bar.py b/src/textual/widgets/_progress_bar.py index 430cd9871d..36597d6141 100644 --- a/src/textual/widgets/_progress_bar.py +++ b/src/textual/widgets/_progress_bar.py @@ -2,14 +2,15 @@ from __future__ import annotations -from time import monotonic -from typing import Callable, Optional +from typing import Optional from rich.style import Style from .._types import UnusedParameter from ..app import ComposeResult, RenderResult +from ..clock import Clock from ..eta import ETA +from ..geometry import clamp from ..reactive import reactive from ..renderables.bar import Bar as BarRenderable from ..widget import Widget @@ -58,8 +59,6 @@ class Bar(Widget, can_focus=False): percentage: reactive[float | None] = reactive[Optional[float]](None) """The percentage of progress that has been completed.""" - _start_time: float | None - """The time when the widget started tracking progress.""" def __init__( self, @@ -67,18 +66,20 @@ def __init__( id: str | None = None, classes: str | None = None, disabled: bool = False, - _get_time: Callable[[], float] = monotonic, + clock: Clock | None = None, ): """Create a bar for a [`ProgressBar`][textual.widgets.ProgressBar].""" - self._get_time = _get_time + self._clock = (clock or Clock()).clone() super().__init__(name=name, id=id, classes=classes, disabled=disabled) - self._start_time = None - self.percentage = None def _validate_percentage(self, percentage: float | None) -> float | None: - """Avoid updating the bar, if the percentage increase is to small to render.""" + """Avoid updating the bar, if the percentage increase is too small to render.""" width = self.size.width * 2 - return None if percentage is None else int(percentage * width) / width + return ( + None + if percentage is None + else (int(percentage * width) / width if width else percentage) + ) def watch_percentage(self, percentage: float | None) -> None: """Manage the timer that enables the indeterminate bar animation.""" @@ -105,11 +106,11 @@ def render(self) -> RenderResult: def render_indeterminate(self) -> RenderResult: """Render a frame of the indeterminate progress bar animation.""" + print(self._clock) width = self.size.width highlighted_bar_width = 0.25 * width # Width used to enable the visual effect of the bar going into the corners. total_imaginary_width = width + highlighted_bar_width - start: float end: float if self.app.animation_level == "none": @@ -118,7 +119,7 @@ def render_indeterminate(self) -> RenderResult: else: speed = 30 # Cells per second. # Compute the position of the bar. - start = (speed * self._get_elapsed_time()) % (2 * total_imaginary_width) + start = (speed * self._clock.time) % (2 * total_imaginary_width) if start > total_imaginary_width: # If the bar is to the right of its width, wrap it back from right to left. start = 2 * total_imaginary_width - start # = (tiw - (start - tiw)) @@ -132,21 +133,6 @@ def render_indeterminate(self) -> RenderResult: background_style=Style.from_color(bar_style.bgcolor), ) - def _get_elapsed_time(self) -> float: - """Get time for the indeterminate progress animation. - - This method ensures that the progress bar animation always starts at the - beginning and it also makes it easier to test the bar if we monkey patch - this method. - - Returns: - The time elapsed since the bar started being animated. - """ - if self._start_time is None: - self._start_time = self._get_time() - return 0 - return self._get_time() - self._start_time - class PercentageStatus(Label): """A label to display the percentage status of the progress bar.""" @@ -242,7 +228,7 @@ def __init__( id: str | None = None, classes: str | None = None, disabled: bool = False, - _get_time: Callable[[], float] = monotonic, + clock: Clock | None = None, ): """Create a Progress Bar widget. @@ -267,22 +253,24 @@ def key_space(self): id: The ID of the widget in the DOM. classes: The CSS classes for the widget. disabled: Whether the widget is disabled or not. + clock: An optional clock object (leave as default unless testing). """ super().__init__(name=name, id=id, classes=classes, disabled=disabled) self.total = total self.show_bar = show_bar self.show_percentage = show_percentage self.show_eta = show_eta - self._get_time = _get_time + self._clock = clock or Clock() self._eta = ETA() def on_mount(self) -> None: self.update() self.set_interval(0.5, self.update) + self._clock.reset() def compose(self) -> ComposeResult: if self.show_bar: - yield Bar(id="bar").data_bind(ProgressBar.percentage) + yield Bar(id="bar", clock=self._clock).data_bind(ProgressBar.percentage) if self.show_percentage: yield PercentageStatus(id="percentage").data_bind(ProgressBar.percentage) if self.show_eta: @@ -300,9 +288,9 @@ def _compute_percentage(self) -> float | None: This will report a percentage of `1` if the total is zero. """ if self.total: - return min(1.0, self.progress / self.total) + return clamp(self.progress / self.total, 0.0, 1.0) elif self.total == 0: - return 1 + return 1.0 return None def advance(self, advance: float = 1) -> None: @@ -340,6 +328,7 @@ def update( progress: Set the progress to the given number of steps. advance: Advance the progress by this number of steps. """ + current_time = self._clock.time if not isinstance(total, UnusedParameter): if total != self.total: self._eta.reset() @@ -351,7 +340,7 @@ def update( if not isinstance(advance, UnusedParameter): self.progress += advance - if self.progress is not None and self.total is not None: - self._eta.add_sample(self._get_time(), self.progress / self.total) + if self.progress is not None and self.total: + self._eta.add_sample(current_time, self.progress / self.total) - self._display_eta = self._eta.get_eta(self._get_time()) + self._display_eta = self._eta.get_eta(current_time) diff --git a/tests/snapshot_tests/__snapshots__/test_snapshots.ambr b/tests/snapshot_tests/__snapshots__/test_snapshots.ambr index f278f078f9..be506192ad 100644 --- a/tests/snapshot_tests/__snapshots__/test_snapshots.ambr +++ b/tests/snapshot_tests/__snapshots__/test_snapshots.ambr @@ -28648,136 +28648,136 @@ font-weight: 700; } - .terminal-2764447286-matrix { + .terminal-2114723073-matrix { font-family: Fira Code, monospace; font-size: 20px; line-height: 24.4px; font-variant-east-asian: full-width; } - .terminal-2764447286-title { + .terminal-2114723073-title { font-size: 18px; font-weight: bold; font-family: arial; } - .terminal-2764447286-r1 { fill: #e1e1e1 } - .terminal-2764447286-r2 { fill: #c5c8c6 } - .terminal-2764447286-r3 { fill: #fea62b } - .terminal-2764447286-r4 { fill: #323232 } - .terminal-2764447286-r5 { fill: #dde8f3;font-weight: bold } - .terminal-2764447286-r6 { fill: #ddedf9 } + .terminal-2114723073-r1 { fill: #e1e1e1 } + .terminal-2114723073-r2 { fill: #c5c8c6 } + .terminal-2114723073-r3 { fill: #fea62b } + .terminal-2114723073-r4 { fill: #323232 } + .terminal-2114723073-r5 { fill: #dde8f3;font-weight: bold } + .terminal-2114723073-r6 { fill: #ddedf9 } - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - IndeterminateProgressBar + IndeterminateProgressBar - + - - - - - - - - - - - - - ━━━━━━━━━━━━╸━━━━━━━━━━━━━━━━━━━39%00:00:07 - - - - - - - - - - - -  S  Start  + + + + + + + + + + + + + ━━━━━━━━━━━━╺━━━━━━━━━━━━━━━━━━━39%00:00:07 + + + + + + + + + + + +  S  Start  @@ -28807,138 +28807,138 @@ font-weight: 700; } - .terminal-3956614203-matrix { + .terminal-1351164996-matrix { font-family: Fira Code, monospace; font-size: 20px; line-height: 24.4px; font-variant-east-asian: full-width; } - .terminal-3956614203-title { + .terminal-1351164996-title { font-size: 18px; font-weight: bold; font-family: arial; } - .terminal-3956614203-r1 { fill: #e1e1e1 } - .terminal-3956614203-r2 { fill: #c5c8c6 } - .terminal-3956614203-r3 { fill: #004578 } - .terminal-3956614203-r4 { fill: #152939 } - .terminal-3956614203-r5 { fill: #1e1e1e } - .terminal-3956614203-r6 { fill: #e1e1e1;text-decoration: underline; } - .terminal-3956614203-r7 { fill: #dde8f3;font-weight: bold } - .terminal-3956614203-r8 { fill: #ddedf9 } + .terminal-1351164996-r1 { fill: #e1e1e1 } + .terminal-1351164996-r2 { fill: #c5c8c6 } + .terminal-1351164996-r3 { fill: #004578 } + .terminal-1351164996-r4 { fill: #152939 } + .terminal-1351164996-r5 { fill: #1e1e1e } + .terminal-1351164996-r6 { fill: #e1e1e1;text-decoration: underline; } + .terminal-1351164996-r7 { fill: #dde8f3;font-weight: bold } + .terminal-1351164996-r8 { fill: #ddedf9 } - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - StyledProgressBar + StyledProgressBar - + - - - - - - - - - - - - - ━━━━━━━━━━━━╸━━━━━━━━━━━━━━━━━━━39%00:00:07 - - - - - - - - - - - -  S  Start  + + + + + + + + + + + + + ━━━━━━━━━━━━╺━━━━━━━━━━━━━━━━━━━39%00:00:07 + + + + + + + + + + + +  S  Start  diff --git a/tests/test_progress_bar.py b/tests/test_progress_bar.py index 3cc3a42bfc..0e9663bf30 100644 --- a/tests/test_progress_bar.py +++ b/tests/test_progress_bar.py @@ -48,7 +48,6 @@ def test_progress_overflow(): pb = ProgressBar(total=100) pb.advance(999_999) - assert pb.progress == 100 assert pb.percentage == 1 pb.update(total=50) @@ -59,7 +58,6 @@ def test_progress_underflow(): pb = ProgressBar(total=100) pb.advance(-999_999) - assert pb.progress == 0 assert pb.percentage == 0 From c796c879606bbc44e20f2e75e238cd0aa41a65e8 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Sun, 10 Mar 2024 15:28:08 +0000 Subject: [PATCH 08/15] remove key, only available in 3.10 --- src/textual/eta.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/textual/eta.py b/src/textual/eta.py index 4f5d8761b7..d18cad1924 100644 --- a/src/textual/eta.py +++ b/src/textual/eta.py @@ -2,7 +2,6 @@ import bisect from math import ceil -from operator import itemgetter from time import monotonic import rich.repr @@ -65,12 +64,12 @@ def _prune(self) -> None: # Keep at least 10 samples return prune_time = self._samples[-1][0] - self.estimation_period - index = bisect.bisect_left(self._samples, prune_time, key=itemgetter(0)) + index = bisect.bisect_left(self._samples, prune_time) del self._samples[:index] def _get_progress_at(self, time: float) -> tuple[float, float]: """Get the progress at a specific time.""" - index = bisect.bisect_left(self._samples, time, key=itemgetter(0)) + index = bisect.bisect_left(self._samples, time) if index >= len(self._samples): return self.last_sample if index == 0: From 7db2f4d20a2d307f722104d70581c5ee80a28cc6 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Mon, 11 Mar 2024 13:41:45 +0000 Subject: [PATCH 09/15] fix bisect call --- src/textual/dom.py | 12 +++++------- src/textual/eta.py | 4 ++-- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/textual/dom.py b/src/textual/dom.py index 94980c87b0..2b47607db4 100644 --- a/src/textual/dom.py +++ b/src/textual/dom.py @@ -474,13 +474,11 @@ def __init_subclass__( cls._merged_bindings = cls._merge_bindings() cls._css_type_names = frozenset(css_type_names) cls._computes = frozenset( - dict.fromkeys( - [ - name.lstrip("_")[8:] - for name in dir(cls) - if name.startswith(("_compute_", "compute_")) - ] - ).keys() + [ + name.lstrip("_")[8:] + for name in dir(cls) + if name.startswith(("_compute_", "compute_")) + ] ) def get_component_styles(self, name: str) -> RenderStyles: diff --git a/src/textual/eta.py b/src/textual/eta.py index d18cad1924..b000979432 100644 --- a/src/textual/eta.py +++ b/src/textual/eta.py @@ -64,12 +64,12 @@ def _prune(self) -> None: # Keep at least 10 samples return prune_time = self._samples[-1][0] - self.estimation_period - index = bisect.bisect_left(self._samples, prune_time) + index = bisect.bisect_left(self._samples, (prune_time, 0)) del self._samples[:index] def _get_progress_at(self, time: float) -> tuple[float, float]: """Get the progress at a specific time.""" - index = bisect.bisect_left(self._samples, time) + index = bisect.bisect_left(self._samples, (time, 0)) if index >= len(self._samples): return self.last_sample if index == 0: From 5a9977e4242a97f3c82a8dd1defda54e227f792d Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Fri, 15 Mar 2024 13:55:07 +0000 Subject: [PATCH 10/15] tests --- src/textual/eta.py | 29 +++++++++----- src/textual/widgets/_progress_bar.py | 11 ++++-- tests/test_eta.py | 56 ++++++++++++++++++++++++++++ 3 files changed, 83 insertions(+), 13 deletions(-) create mode 100644 tests/test_eta.py diff --git a/src/textual/eta.py b/src/textual/eta.py index b000979432..ffaaa1f0a7 100644 --- a/src/textual/eta.py +++ b/src/textual/eta.py @@ -11,13 +11,17 @@ class ETA: """Calculate speed and estimate time to arrival.""" - def __init__(self, estimation_period: float = 60) -> None: + def __init__( + self, estimation_period: float = 60, extrapolate_period: float = 30 + ) -> None: """Create an ETA. Args: - estimation_period: Period in seconds, used to calculate speed. Defaults to 30. + estimation_period: Period in seconds, used to calculate speed. + extrapolate_period: Maximum number of seconds used to estimate progress after last sample. """ self.estimation_period = estimation_period + self.max_extrapolate = extrapolate_period self._samples: list[tuple[float, float]] = [(0.0, 0.0)] self._add_count = 0 @@ -51,7 +55,6 @@ def add_sample(self, time: float, progress: float) -> None: if self._samples and self.last_sample[1] > progress: # If progress goes backwards, we need to reset calculations self.reset() - return self._samples.append((time, progress)) self._add_count += 1 if self._add_count % 100 == 0: @@ -69,14 +72,15 @@ def _prune(self) -> None: def _get_progress_at(self, time: float) -> tuple[float, float]: """Get the progress at a specific time.""" + index = bisect.bisect_left(self._samples, (time, 0)) if index >= len(self._samples): return self.last_sample if index == 0: return self.first_sample # Linearly interpolate progress between two samples - time1, progress1 = self._samples[index] - time2, progress2 = self._samples[index + 1] + time1, progress1 = self._samples[index - 1] + time2, progress2 = self._samples[index] factor = (time - time1) / (time2 - time1) intermediate_progress = progress1 + (progress2 - progress1) * factor return time, intermediate_progress @@ -86,7 +90,7 @@ def speed(self) -> float | None: """The current speed, or `None` if it couldn't be calculated.""" if len(self._samples) < 2: - # Need at less 2 samples to calculate speed + # Need at least 2 samples to calculate speed return None recent_sample_time, progress2 = self.last_sample @@ -109,7 +113,14 @@ def get_eta(self, time: float) -> int | None: # Not enough samples to guess return None recent_time, recent_progress = self.last_sample - time_since_sample = time - recent_time - remaining = 1.0 - (recent_progress + speed * time_since_sample) - eta = max(0, remaining / speed) + remaining = 1.0 - recent_progress + if remaining <= 0: + # Complete + return 0 + # The bar is not complete, so we will extrapolate progress + # This will give us a countdown, even with no samples + time_since_sample = min(self.max_extrapolate, time - recent_time) + extrapolate_progress = speed * time_since_sample + # We don't want to extrapolate all the way to 0, as that would erroneously suggest it is finished + eta = max(1.0, (remaining - extrapolate_progress) / speed) return ceil(eta) diff --git a/src/textual/widgets/_progress_bar.py b/src/textual/widgets/_progress_bar.py index 36597d6141..828fb02426 100644 --- a/src/textual/widgets/_progress_bar.py +++ b/src/textual/widgets/_progress_bar.py @@ -106,7 +106,6 @@ def render(self) -> RenderResult: def render_indeterminate(self) -> RenderResult: """Render a frame of the indeterminate progress bar animation.""" - print(self._clock) width = self.size.width highlighted_bar_width = 0.25 * width # Width used to enable the visual effect of the bar going into the corners. @@ -334,13 +333,17 @@ def update( self._eta.reset() self.total = total + def add_sample() -> None: + """Add a new sample.""" + if self.progress is not None and self.total: + self._eta.add_sample(current_time, self.progress / self.total) + if not isinstance(progress, UnusedParameter): self.progress = progress + add_sample() if not isinstance(advance, UnusedParameter): self.progress += advance - - if self.progress is not None and self.total: - self._eta.add_sample(current_time, self.progress / self.total) + add_sample() self._display_eta = self._eta.get_eta(current_time) diff --git a/tests/test_eta.py b/tests/test_eta.py new file mode 100644 index 0000000000..c91903293b --- /dev/null +++ b/tests/test_eta.py @@ -0,0 +1,56 @@ +from textual.eta import ETA + + +def test_basics() -> None: + eta = ETA() + eta.add_sample(1.0, 1.0) + assert eta.first_sample == (0, 0) + assert eta.last_sample == (1.0, 1.0) + assert len(eta._samples) == 2 + repr(eta) + + +def test_speed() -> None: + eta = ETA() + # One sample is not enough to determine speed + assert eta.speed is None + eta.add_sample(1.0, 0.5) + assert eta.speed == 0.5 + + # Check reset + eta.reset() + assert eta.speed is None + eta.add_sample(0.0, 0.0) + assert eta.speed is None + eta.add_sample(1.0, 0.5) + assert eta.speed == 0.5 + + # Check backwards + eta.add_sample(2.0, 0.0) + assert eta.speed is None + eta.add_sample(3.0, 1.0) + assert eta.speed == 1.0 + + +def test_get_progress_at() -> None: + eta = ETA() + eta.add_sample(1, 2) + # Check interpolation works + assert eta._get_progress_at(0) == (0, 0) + assert eta._get_progress_at(0.1) == (0.1, 0.2) + assert eta._get_progress_at(0.5) == (0.5, 1.0) + + +def test_eta(): + eta = ETA(estimation_period=2, extrapolate_period=5) + eta.add_sample(1, 0.1) + assert eta.speed == 0.1 + assert eta.get_eta(1) == 9 + assert eta.get_eta(2) == 8 + assert eta.get_eta(3) == 7 + assert eta.get_eta(4) == 6 + assert eta.get_eta(5) == 5 + # After 5 seconds (extrapolate_period), eta won't update + assert eta.get_eta(6) == 4 + assert eta.get_eta(7) == 4 + assert eta.get_eta(8) == 4 From c062b8e51adc9e88cfdc79fa69210245a291ef26 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Mon, 18 Mar 2024 12:11:52 +0000 Subject: [PATCH 11/15] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 14b45d4618..963f4eb491 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Changed `Tabs` - Changed `TextArea` - Changed `Tree` +- Improved ETA calculation for ProgressBar https://github.com/Textualize/textual/pull/4271 ## [0.52.1] - 2024-02-20 From 88d15244bdfa9165d0e0214365389064a3428f72 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Mon, 18 Mar 2024 13:02:01 +0000 Subject: [PATCH 12/15] watch progress --- src/textual/widgets/_progress_bar.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/textual/widgets/_progress_bar.py b/src/textual/widgets/_progress_bar.py index 828fb02426..007188ad56 100644 --- a/src/textual/widgets/_progress_bar.py +++ b/src/textual/widgets/_progress_bar.py @@ -292,6 +292,10 @@ def _compute_percentage(self) -> float | None: return 1.0 return None + def _watch_progress(self, progress: float) -> None: + """Perform update when progress is modified.""" + self.update(progress=progress) + def advance(self, advance: float = 1) -> None: """Advance the progress of the progress bar by the given amount. From 71fc96c1f90a6508c71a52f27458b3b7ea7aadd9 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Mon, 18 Mar 2024 13:13:06 +0000 Subject: [PATCH 13/15] fix reset --- src/textual/widgets/_progress_bar.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/textual/widgets/_progress_bar.py b/src/textual/widgets/_progress_bar.py index 007188ad56..0ec18735f0 100644 --- a/src/textual/widgets/_progress_bar.py +++ b/src/textual/widgets/_progress_bar.py @@ -333,7 +333,7 @@ def update( """ current_time = self._clock.time if not isinstance(total, UnusedParameter): - if total != self.total: + if total is None or total != self.total: self._eta.reset() self.total = total @@ -350,4 +350,6 @@ def add_sample() -> None: self.progress += advance add_sample() - self._display_eta = self._eta.get_eta(current_time) + self._display_eta = ( + None if self.total is None else self._eta.get_eta(current_time) + ) From 7b118576d91d4227aff9a0715f29755969ab1af7 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Mon, 18 Mar 2024 13:20:25 +0000 Subject: [PATCH 14/15] update less often --- src/textual/widgets/_progress_bar.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/textual/widgets/_progress_bar.py b/src/textual/widgets/_progress_bar.py index 0ec18735f0..1eef38dc05 100644 --- a/src/textual/widgets/_progress_bar.py +++ b/src/textual/widgets/_progress_bar.py @@ -264,7 +264,7 @@ def key_space(self): def on_mount(self) -> None: self.update() - self.set_interval(0.5, self.update) + self.set_interval(1, self.update) self._clock.reset() def compose(self) -> ComposeResult: From 1c7c1c2e2e487176a673431196b865e1a943b9c2 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Mon, 18 Mar 2024 13:36:45 +0000 Subject: [PATCH 15/15] test fix --- src/textual/eta.py | 4 ++-- src/textual/widgets/_progress_bar.py | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/textual/eta.py b/src/textual/eta.py index ffaaa1f0a7..f6036bddac 100644 --- a/src/textual/eta.py +++ b/src/textual/eta.py @@ -31,13 +31,13 @@ def __rich_repr__(self) -> rich.repr.Result: @property def first_sample(self) -> tuple[float, float]: - """First sample, or `None` if no samples.""" + """First sample.""" assert self._samples, "Assumes samples not empty" return self._samples[0] @property def last_sample(self) -> tuple[float, float]: - """Last sample, or `None` if no samples.""" + """Last sample.""" assert self._samples, "Assumes samples not empty" return self._samples[-1] diff --git a/src/textual/widgets/_progress_bar.py b/src/textual/widgets/_progress_bar.py index 1eef38dc05..b53a31c7c3 100644 --- a/src/textual/widgets/_progress_bar.py +++ b/src/textual/widgets/_progress_bar.py @@ -163,7 +163,7 @@ class ETAStatus(Label): } """ eta: reactive[float | None] = reactive[Optional[float]](None) - """Estimated number of seconds till completion.""" + """Estimated number of seconds till completion, or `None` if no estimate is available.""" def render(self) -> RenderResult: """Render the ETA display.""" @@ -254,13 +254,13 @@ def key_space(self): disabled: Whether the widget is disabled or not. clock: An optional clock object (leave as default unless testing). """ + self._clock = clock or Clock() + self._eta = ETA() super().__init__(name=name, id=id, classes=classes, disabled=disabled) self.total = total self.show_bar = show_bar self.show_percentage = show_percentage self.show_eta = show_eta - self._clock = clock or Clock() - self._eta = ETA() def on_mount(self) -> None: self.update()