test(plotter): exercise the swallow-and-continue paths, and say pass - #2095
Merged
Merged
Conversation
PlotterBase used a bare `1` as a no-op statement in nine places, which is what `reportUnusedExpression` was flagging. Replacing it with `pass` is inert -- the file is AST-identical once `1`-as-statement is normalized -- so the change worth making alongside it is covering what those guards actually promise. The five maybe_* helpers (cudf, dask_cudf, dask.dataframe, pyspark, polars) are the contract engine dispatch is built on: each must return None when its library is absent rather than raise. That is now pinned in both directions, plus the lru_cache that makes them cheap enough to call per dispatch decision. Two `except:` guards protect an upload from an advisory step: a failure inside infer_labels, and an edges table whose len() raises. Both are pinned by tests that fail when the guard is removed -- verified by removing each one. Two of the nine no-ops sat directly after a `logger.debug(...)`, so they were redundant rather than required; those are deleted instead of converted, which also keeps them out of the changed-line coverage gate. The remaining bare-`1` sites (gremlin, layouts, cugraph, igraph) are left for the PRs that own those files. cugraph's three are unreachable on a CPU lane, so they need the GPU-lane treatment rather than a cosmetic sweep here. Pyright ratchet 271 -> 262 (-9). Changed-line coverage 7/7. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017ropeBMLJUuy6ViYwy15ud
test_polars_lane_completeness caught it: a test module that mentions polars must either run in the polars lane or be justified as not needing one. This module asserts maybe_polars returns the module when polars is present, so it wants the lane -- without it that half of the pin never executes. Under polars it goes from 8 passed / 5 skipped to 9 passed / 4 skipped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017ropeBMLJUuy6ViYwy15ud
This was referenced Sep 18, 2026
lmeyerov
added a commit
that referenced
this pull request
Sep 18, 2026
from_igraph's third branch, which un-swizzles integer vertex IDs, recorded success in a separate `found` boolean: node_id_col was bound inside the loop and read after it, with `if not found` as the only thing tying the two together. The name is in fact always bound -- every arm of the chain either assigns it or raises -- but nothing local says so, which is why it read as possibly unbound. Binding node_id_col = None up front and testing that instead says the same thing with one name rather than two, and the flag disappears. Behaviour verified rather than assumed: round-trips through igraph produce byte-identical edge frames on master and here for all three shapes that reach different arms -- named vertices, integer vids (the arm this touches), and a bare attribute-less graph. 37 igraph tests, 50 plugin tests. Also `pass` for a bare `1` used as a no-op, the same idiom cleared from PlotterBase in #2095. cugraph's findings are not here: they are GPU-only, so no lane feeding the changed-line gate can execute them, and they want dgx validation of their own. Pyright ratchet 245 -> 240 (-5). Changed-line coverage 3/3. Claude-Session: https://claude.ai/code/session_017ropeBMLJUuy6ViYwy15ud Co-authored-by: Claude Opus 5 (1M context) <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 free
to 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.
Third of the pyright-ratchet cleanup series (#2092 gate, #2093 layout, #2094 outliers). Ratchet 271 → 262.
What this started as, and why it changed
PlotterBaseused a bare1as a no-op statement in nine places — someone's stand-in forpass, which is whatreportUnusedExpressionwas flagging. Swapping it forpassis inert: every touched file is AST-identical once1-as-statement is normalized toPass.But a purely cosmetic change drags nine previously-unexecuted lines into the changed-line-coverage gate, and measurement showed how unexecuted they were: across the full suite (8530 tests), 1 of the 15 bare-
1sites repo-wide runs. So the change worth making alongside the rename is covering what those guards actually promise.What is now pinned
The five
maybe_*optional-import helpers (cudf, dask_cudf, dask.dataframe, pyspark, polars) are the contract engine dispatch is built on: each must returnNonewhen its library is absent rather than raise. Pinned in both directions, plus thelru_cachethat makes them cheap enough to call per dispatch decision.Two
except:guards protect an upload from an advisory step — a failure insideinfer_labels, and an edges table whoselen()raises. Both are pinned by tests that fail when the guard is removed; I verified that by removing each one and watching the matching test go red.This is the same defect class as #2094, where an optional-import guard turned out not to work at all. These five do work — now there is a test saying so.
Two deletions rather than conversions
Two of the nine no-ops sat directly after a
logger.debug(...), so they were redundant rather than required. Deleting them is more honest than converting them, and it keeps them out of the coverage gate.Deliberately not here
The other six bare-
1sites (gremlin.py,layouts.py,plugins/cugraph.py×3,plugins/igraph.py) are left for the PRs that own those files. cugraph's three are unreachable on a CPU lane, so they need the GPU-lane treatment rather than a cosmetic sweep bundled in here.Verification
bin/lint.sh(ruff, type-hygiene 4448, comment-density 1151) · pyright guard 262 · mypy 351 files · changed-line coverage 7/7 = 100% · 72 plotter tests.Unrelated observation while running the full suite:
test_temporal_leak_family_1915.py::TestB7Units::test_align_mixed_tz_converts_only_mixed_datetime_pairsfails on cleanmasterunder pandas 3.0.5. Not touched by this PR; flagging it separately.🤖 Generated with Claude Code
https://claude.ai/code/session_017ropeBMLJUuy6ViYwy15ud