Stop losing and double-counting lap time - #7
Merged
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Three ways the stopwatch reported wrong numbers without ever raising. `lap()` was a @contextmanager with a bare `yield` between `start()` and `stop()`, so an exception inside the block skipped `stop()` entirely. The lap stayed open forever, and because `Lap.elapsed` falls back to `perf_counter() - self._start` while running, `elapsed`, `statistics` and `report()` then returned a different value on every read. Measured before the fix: 0.0502 then 0.1004 from two consecutive reads of the same stopwatch. Now wrapped in try/finally. Nested `lap()` calls silently discarded the outer lap's time. `start()` returns early when the stopwatch is already running, so the inner call opened no lap, and the inner `stop()` closed the *outer* one -- leaving the outer `stop()` a no-op and everything after the inner block unmeasured. A 0.25s outer block containing a 0.10s inner block reported 0.15s in a single lap. Each `lap()` now owns a distinct lap. The reuse of the running lap is deliberate and kept: a Stopwatch starts measuring on construction, so the first `lap()` adopts that lap rather than opening a second one, which is what stops `with Stopwatch() as sw: with sw.lap():` from recording a phantom lap. Clearing `_current_lap` is what hands ownership to the context manager and lets a nested call open its own lap. What remains is noted as a ponytail comment on `lap()`: time between construction and the first `lap()` is still billed to that first lap. `running` now asks the laps instead of `_current_lap`, which is None while a `lap()` block owns its lap. `Lap.stop()` is idempotent. It used to append `perf_counter() - self._start` unconditionally after having reset `_start` to 0.0, so a second call recorded the machine uptime as a lap fraction. Not reachable through Stopwatch today, but `Lap` is importable and the failure mode is silent garbage rather than an error. Each fix has a regression test. Verified they catch the bugs by reverting the three fixes and re-running: exactly those three tests fail, the other 35 pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
devRMAforce-pushed
the
fix/timing-correctness
branch
from
August 2, 2026 19:52
179a96d to
e9cf30aCompareDrop the inline comments and the ponytail marker, and make the lap handover legible on its own: `_take_running_lap() or _open_lap()` says what the branch did. `_open_lap` also removes the duplicated append/start that `start()` was repeating. The remaining docstrings state behaviour only, no rationale. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A stopwatch starts measuring on construction, and the first lap() takes over that lap, so setup work between the constructor and the first lap() is billed to it: sw = Stopwatch() load_config() # 5s for item in items: with sw.lap(): process(item) # 0.1s -> first lap reports 5.1s `autostart=False` skips the initial start(), so only the blocks wrapped in lap() are measured. The default stays True, so nothing changes for existing callers, and using the stopwatch as a context manager still starts it because __enter__ restarts regardless. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Three ways the stopwatch reported wrong numbers without ever raising.
lap()was a @contextmanager with a bareyieldbetweenstart()andstop(), so an exception inside the block skippedstop()entirely.The lap stayed open forever, and because
Lap.elapsedfalls back toperf_counter() - self._startwhile running,elapsed,statisticsand
report()then returned a different value on every read. Measuredbefore the fix: 0.0502 then 0.1004 from two consecutive reads of the
same stopwatch. Now wrapped in try/finally.
Nested
lap()calls silently discarded the outer lap's time.start()returns early when the stopwatch is already running, so the inner call
opened no lap, and the inner
stop()closed the outer one -- leavingthe outer
stop()a no-op and everything after the inner blockunmeasured. A 0.25s outer block containing a 0.10s inner block reported
0.15s in a single lap. Each
lap()now owns a distinct lap.The reuse of the running lap is deliberate and kept: a Stopwatch starts
measuring on construction, so the first
lap()adopts that lap ratherthan opening a second one, which is what stops
with Stopwatch() as sw: with sw.lap():from recording a phantom lap._take_running_lap()is what hands ownership to the context manager andlets a nested call open its own lap.
Known limitation, documented in the
lap()docstring: time betweenconstruction and the first
lap()is still billed to that first lap.Call
reset()first to leave it out. Making this go away means givingStopwatchanautostart=Falseoption, which is an API decision leftopen deliberately.
runningnow asks the laps instead of_current_lap, which is Nonewhile a
lap()block owns its lap.Lap.stop()is idempotent. It used to appendperf_counter() - self._startunconditionally after having reset_startto 0.0, so asecond call recorded the machine uptime as a lap fraction. Not reachable
through Stopwatch today, but
Lapis importable and the failure mode issilent garbage rather than an error.
Each fix has a regression test. Verified they catch the bugs by
reverting the three fixes and re-running: exactly those three tests
fail, the other 35 pass.
Co-Authored-By: Claude Opus 5 noreply@anthropic.com
Stack created with GitHub Stacks CLI • Give Feedback 💬