refactor(typing): canonical polars frame vocabulary + is_polars_df TypeGuard (Wave 0) - #1799
Open
lmeyerov wants to merge 4 commits into
Open
refactor(typing): canonical polars frame vocabulary + is_polars_df TypeGuard (Wave 0)#1799lmeyerov wants to merge 4 commits into
lmeyerov wants to merge 4 commits into
Conversation
This was referenced Jul 28, 2026
lmeyerov
added a commit
that referenced
this pull request
Jul 28, 2026
…erywhere (#1805) * test(gfql): make the polars lane see the 280 tests it was skipping everywhere polars is installed in exactly ONE CI lane (test-polars, driven by bin/test-polars.sh), and that lane runs a hand-maintained file list. Every other lane collection-skips a module-level `pytest.importorskip("polars")` and every polars/polars-gpu parameter. A polars-gated test the list omits therefore runs NOWHERE while both lanes report green. Measured on run 30311675717 (PR #1799): 280 tests executed locally with polars installed and appear in ZERO job logs of that run, including six whole modules — four of them named `test_engine_polars_*`. Two real defects were sitting in the blind spot: - `test_explicit_polars_gpu_declines_indexed_helper_and_falls_back` executes engine='polars-gpu' but only gates on `importorskip("polars")`, so it raises ImportError wherever polars is present without the RAPIDS cudf_polars stack. It has never once run: 9 SKIPPED, 0 executed, across the whole run. - `test_case_regex_unicode_trick_matrix[toupper-eq-ss-fold]` and `[tolower-turkish-dotted-i]` catch a genuine silent cross-engine divergence: under pandas>=3 the pandas engine answers [9] where polars answers [8, 9] for `toUpper(n.name) = 'STRASSE'`. Filed as #1802 with the repro; both rows are marked xfail(strict=True) keyed on the pandas accessor's own behaviour, so the fix XPASSes and retires the marks. Changes: - bin/test-polars.sh gains the six invisible modules plus the four whose polars parameters were invisible. - New `test_polars_lane_completeness.py` parses POLARS_TEST_FILES and fails when a polars-mentioning test module is neither in the lane nor in a documented exemption, when a module-level polars gate sits outside the lane, when a listed path is gone, or when a polars-gated test in the `-k polars` phase is unselectable by that filter. Mutation-checked: dropping one entry turns two of its assertions red. - Split the polars-gpu test so the CPU half (helper must decline) runs wherever polars does, and the execution half skips honestly without cudf_polars. - Renamed the two test_lowering.py polars tests `-k polars` could not select. Test-only; no product code touched, so no CHANGELOG entry. Widened lane, py3.11 + polars 1.43.1 + pandas 3.0.5: 2726 passed, 142 skipped, 3 xfailed, 0 failed (was 1636 passed on the same box). ORDERING NOTE: #1797 records that the py3.12 coverage cell is already at its 10-minute ceiling and that adding a single file tipped it into a timeout; the pending ci.yml split (comment on PR #1794) must land BEFORE this. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015YsqAZQLbqjSDrYSFz2GoB * test(gfql): granular AST types in the polars-lane completeness guard Replace the getattr/ast.AST pairing in the guard's two helpers with the concrete node types they are actually called with: _node_source takes ast.stmt | ast.expr and reads .lineno/.end_lineno directly (end_lineno is Optional[int], so the None case is handled rather than defaulted through getattr), and _selected_by_k_polars takes a FuncDef alias so .name and .decorator_list are checked attributes instead of getattr lookups with invented fallbacks. No behaviour change; guard still 8 passed, ruff clean, mypy clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015YsqAZQLbqjSDrYSFz2GoB --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…peGuard
Typing Wave 0. No runtime behaviour change, no public API change.
W0.1 -- graphistry/compute/typing.py gains PolarsFrame (pl.DataFrame |
pl.LazyFrame), a constrained PolarsT TypeVar and PolarsSeriesT, defined
TYPE_CHECKING-only so polars stays optional and no runtime import or
typing-extensions floor is added. polars/dtypes.py, which had defined
PolarsFrame/PolarsT first, now re-exports them so there is ONE definition.
DataFrameT is deliberately NOT widened: turning it into a cross-engine union
fans out across all 325 checked modules. The fix for a polars-only helper is
a new name, not a blurred old one.
W0.2 -- Engine.is_polars_df becomes
(df: object) -> TypeGuard["PolarsFrame"]
so the polars branch of an engine-dispatching helper is CHECKED against the
polars API instead of silently accepted. TypeGuard, NOT TypeIs (PEP 742):
TypeIs also narrows the negative branch, and to do that soundly it requires
the narrowed type to be consistent with the declared input type -- a polars
frame is not a subtype of the pandas type these call sites declare. (That is
the difference from dtypes.is_lazy, whose input is already a PolarsFrame.)
A companion is_polars_series -- same import-light module check -- narrows the
SERIES call sites to pl.Series, which the frame guard would have mistyped.
Fallout, all typing-only:
- The six per-branch ignores in Engine.py's dispatch block were written
against an older stub set and named codes (attr-defined, no-any-return)
that no longer applied: they were DEAD, suppressing nothing. They are now
scoped to the one thing that genuinely cannot typecheck -- the polars value
flowing out of a DataFrameT/SeriesT signature.
- _pl_nan_to_null takes the PolarsFrame union instead of the constrained
PolarsT TypeVar, which a union argument cannot bind. An @overload set would
keep per-flavour precision but is unusable: on the polars-less type-lint
lane every signature collapses to Any -> Any and mypy rejects the set.
- Two rebindings that mixed a polars result into a pandas-typed local
(gfql/row/frame_ops.py rows(), gfql_unified.py optional-match seed) now
return / branch directly instead. Same calls, same arguments, same results.
- Two ignores in frame_ops.count_table are removed: the guard now narrows, so
collect_schema()/select() typecheck on their own.
Measured, same tree, three environments:
isolated (what CI runs today): no issues in 325 files, before AND after
stub-visible + polars importable: 163 in 11 files -> 157 in 9 files
stub-visible, polars absent: 128 in 8 files -> 124 in 6 files
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015YsqAZQLbqjSDrYSFz2GoB
lmeyerov
force-pushed
the
typing/wave0-polars-frame
branch
from
July 28, 2026 03:01
20876e5 to
578c677
Compare
Conflicting files: CHANGELOG.md (both sides prepended to '### Changed'; took master's file and re-inserted this branch's entry) and graphistry/compute/gfql/lazy/engine/polars/row_pipeline.py (master's #1794 bounded var-length decline gate + this branch's is_polars_df TypeGuard retype; disjoint hunks, both kept). No substance change to the branch. Verified on the merged tree: ruff clean repo-wide, mypy clean repo-wide with polars absent (the config the python-lint-types lane runs), and with polars stubs visible 4 -> 2 errors vs master (both remaining are pre-existing in degrees.py, untouched here).
Only CHANGELOG.md changed on both sides. Resolved structurally, not textually: took master#\s file whole and re-inserted this branch#\s single ### Changed entry at the top of that section, so the diff vs master is +1/-0 on CHANGELOG. The eight source files this branch retypes were untouched by #1811 (which only moved test_index.py and the CHANGELOG), so they carry through unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YYZRXegrALuXd3NHH5evqx
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.
Typing Wave 0 (W0.1 + W0.2). Typing only — no runtime behaviour change, no public API change. W0.3 (the
bin/typecheck.sh/ lockfile /ci.ymlgate fix) is deliberately NOT in this PR.Reproduced baseline (exact commands,
origin/master=233b64c8)Three environments, same tree:
uvx mypy, isolated — no pandas-stubs, no polars)MYPY_CMD="uvx mypy" ./bin/mypy.shSuccess: no issues found in 325 source filesmypy --config-file mypy.ini graphistryin a venv withmypy pandas-stubs types-defusedxml types-requests types-tqdmpolarsAll three reproduced exactly (mypy 2.3.0, py3.13).
What changed
W0.1 —
graphistry/compute/typing.pygains a canonical engine-frame vocabulary:PolarsFrame(pl.DataFrame | pl.LazyFrame), a constrainedPolarsTTypeVar, andPolarsSeriesT. DefinedTYPE_CHECKING-only (the shape already proven inpolars/dtypes.py), so polars stays an optional dependency and no runtime import ortyping-extensionsversion floor is introduced.polars/dtypes.py, which had definedPolarsFrame/PolarsTfirst, now re-exports them so there is ONE definition.DataFrameTis not widened. Turning it into a cross-engine union fans out across all 325 checked modules; the fix for a polars-only helper is a NEW name, not a blurred old one.W0.2 —
graphistry/Engine.py:is_polars_df(df: Any) -> boolbecomesis_polars_df(df: object) -> TypeGuard["PolarsFrame"].TypeGuard, notTypeIs(PEP 742) — and this reasoning should survive review:TypeIsadditionally narrows the negative branch, and to do that soundly it requires the narrowed type to be consistent with the declared input type. These call sites declareDataFrameT/SeriesT(pandas at checking time), and a polars frame is not a subtype of a pandas one, soTypeIsis rejected outright. That is exactly what distinguishes this fromdtypes.is_lazy(whose input is already aPolarsFrame, soTypeIsis correct there) and from #1789.A companion
is_polars_series— the identical import-light module check — narrows the SERIES call sites topl.Series. Without it the frame guard would promise.height/.columnson a value that is a column.Dead ignores removed. The six per-branch
# type: ignore[attr-defined,no-any-return]comments inEngine.py's dispatch block were written against an older stub set; the real codes are[operator]/[return-value], so those ignores were suppressing nothing. They are now scoped to the one thing that genuinely cannot typecheck: the polars value flowing out of aDataFrameT/SeriesTsignature. Two more (frame_ops.count_table) are deleted outright — the guard now narrows, socollect_schema()/select()typecheck on their own.PolarsTover-constraint._pl_nan_to_nullnow takes/returns thePolarsFrameunion rather than the constrained TypeVar, which a union argument cannot bind. An@overloadset would keep per-flavour precision, and I tried it — it is unusable here: on the polars-less type-lint lane every signature collapses toAny -> Anyand mypy rejects the set as unmatchable (overload-cannot-match), which turned the currently-green isolated CI lane red. Recorded in the docstring so it is not re-attempted.Two rebindings restructured (
gfql/row/frame_ops.py:rows,gfql_unified.pyoptional-match seed): they mixed a polars result into a pandas-typed local, which widened the variable and broke the pandas branch below (TypeGuardnarrows only the positive branch). Both now return / branch directly — same calls, same arguments, same results.Re-measured residual — and the honest finding
The payoff is 6, not the projected ~24 — and the projection's premise was wrong, not merely optimistic. BW4 expected ~24 errors to evaporate across
Engine.py,gfql/index/traverse.py,gfql/index/engine_arrays.py,dataframe/join.py,gfql_fast_paths.pyandgfql_unified.pywith no per-site edit. Measured: four of those six files never callis_polars_dfat all.traverse.py(8),engine_arrays.py(4) anddataframe/join.py(4) have zero call sites;gfql_fast_paths.pycalls it twice, for a plain boolean, and none of its 108 errors touch it. Their error counts are byte-for-byte unchanged by this PR, because those errors come from unguarded polars-only helpers annotatedDataFrameT— Wave 1 per-file work, which W0.1's new vocabulary now exists to serve. The narrowing only ever reachedEngine.py(5) andgfql_unified.py(1).There is also a second-order effect worth naming:
Plottable._edges/._nodesare declaredAny, so the TypeGuard narrowed previously-uncheckedAnyand surfaced 9 new errors inComputeMixin.py,frame_ops.py,row_pipeline.pyandgfql_unified.pythat master never reported. All are resolved here (that is why the intermediate measurement went 163 → 179 before coming back down to 157). This is more checking, not less, but it means Wave 1's sizing should assume narrowing reveals work as well as removing it.Verification
bin/lint.sh— clean.233b64c8with the same env (test_explicit_polars_gpu_declines_indexed_helper_and_falls_back, twotest_case_regex_unicode_trick_matrixparams) — pre-existing, not caused by this change.df_to_engine(pa.Table -> POLARS), and a polarschain/gfqlround trip.--warn-unused-ignoresdiffed against baseline: no newly-dead ignores introduced (the only deltas are line shifts of pre-existing ones); 7 dead ignores retired.Untouched, per scope:
bin/, lockfiles, workflows, andgfql_fast_paths.py.🤖 Generated with Claude Code
https://claude.ai/code/session_015YsqAZQLbqjSDrYSFz2GoB