fix(layout): bind the locals these three paths read on every route - #2096
Merged
Merged
Conversation
Each of these read a name that an earlier branch may never have bound, so the failure mode was UnboundLocalError rather than whatever was intended. GraphBase.__init__ reads the component `s` in its connectivity check, but `s` is only assigned inside the edge loop. With no edges the check still runs; it is saved today only by short-circuiting on `v.component is None`, which stops holding as soon as a caller passes vertices that already carry a component. Initializing `s = None` preserves the behaviour exactly -- a vertex whose component is not None no longer matches `s`, so it is still reported unconnected -- and removes the crash. tangents() repeated the final tangent by appending the loop variable after the loop, which needs the loop to have run. It now appends T[-1]. Verified identical to the old function on every input tried. mercator_layout bound cupy only inside its import guard and gated the GPU branch on a separate use_cupy flag; `cp is not None` says the same thing with one name instead of two. The cupy branch was verified equal to the numpy branch to 1e-9 on a GB10 (cudf 26.02.01, cupy 13.6.0), including the equator and the +/-89 degree cases. Its two cuDF-only lines carry the established pragma, since no lane feeding the changed-line gate can execute them. Five tests; the two that can be mutated were checked by mutating them. Pyright ratchet 262 -> 255 (-7). Changed-line coverage 4/4. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017ropeBMLJUuy6ViYwy15ud
8 tasks
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fourth of the pyright-ratchet cleanup series (#2092 gate, #2093 layout, #2094 outliers, #2095 plotter). Ratchet 262 → 255.
Each of these reads a name that an earlier branch may never have bound, so the failure mode was
UnboundLocalErrorrather than whatever was intended.GraphBase.__init__— the one with teethsis assigned only inside the edge loop, but the connectivity check after it readssunconditionally. With no edges, nothing binds it.Today that is saved by short-circuiting: the check is
v.component is None or (v.component != s), and with no edges every vertex hascomponent is None, sosis never evaluated. That stops holding the moment a caller passes vertices that already carry a component — then the first vertex reachesv.component != sand raisesUnboundLocalError.s = Nonepreserves the behaviour exactly: a vertex whose component is notNonestill fails!= s, so it is still reported unconnected. Pinned both ways, including the single-vertex short circuit.tangents()Repeated the final tangent by appending the loop variable after the loop, which requires the loop to have run — guaranteed only by an
assert n >= 2, and asserts vanish under-O. Now appendsT[-1], which also states the intent. Verified identical to the old function on every input tried (n = 2, 3, 5, 9).mercator_layoutBound
cupyonly inside its import guard and gated the GPU branch on a separateuse_cupyflag.cp is not Nonesays the same thing with one name instead of two.GPU validation (this changes which branch runs, so it is not a CPU-only claim): on a GB10 with cudf 26.02.01 / cupy 13.6.0, the cupy branch equals the numpy branch to 1e-9 across seven points including the equator, ±89° latitude and ±179.9° longitude.
Its two cuDF-only lines carry the established
# pragma: no cover - cuDF-only ... (validated on dgx), since no lane feeding the changed-line gate can execute them.A smaller form that avoided those two lines entirely (
cp = Nonethenimport cupy as cp) was tried and rejected: mypy reportsno-redef.Verification
bin/lint.sh(ruff, type-hygiene 4448, comment-density 1151) · pyright guard 255 · mypy 351 files · changed-line coverage 4/4 = 100% · 251 layout tests · the meta-guards, since this adds a test file · both mutable tests confirmed discriminating by mutating the code under them.🤖 Generated with Claude Code
https://claude.ai/code/session_017ropeBMLJUuy6ViYwy15ud