You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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).
Copy file name to clipboardExpand all lines: CHANGELOG.md
+3Lines changed: 3 additions & 0 deletions
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -26,10 +26,13 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
26
26
27
27
### Changed
28
28
- **`is_polars_df` is a type predicate, and the engine-frame vocabulary has a canonical home**: `graphistry/compute/typing.py` now exports `PolarsFrame` (the `pl.DataFrame | pl.LazyFrame` union), a `PolarsT` TypeVar and `PolarsSeriesT`, TYPE_CHECKING-only so polars stays an optional dependency and no runtime import or version floor is added; `polars/dtypes.py`, which had defined the first two, re-exports them so there is ONE definition. `DataFrameT` is deliberately left pinned to `pd.DataFrame` — widening it into a cross-engine union fans out across every checked module, and the fix for a polars-only helper is to name polars, not to blur the pandas alias. Against that vocabulary `Engine.is_polars_df` is now declared `(df: object) -> TypeGuard["PolarsFrame"]` instead of `(df: Any) -> bool`, so the polars branch of an engine-dispatching helper is actually CHECKED against the polars API rather than silently accepted. `TypeGuard`, not `TypeIs` (PEP 742): `TypeIs` also narrows the negative branch, but to do that soundly it requires the narrowed type to be consistent with the declared input, and 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` (the same import-light module check) narrows the SERIES call sites to `pl.Series`, which the frame guard would have mistyped. Consequences: the six per-branch `# type: ignore` comments 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, and are now scoped to the one thing that genuinely cannot typecheck, the polars return 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 the per-flavour precision but is unusable here — on the polars-less type-lint lane every signature collapses to `Any -> Any`); and two rebindings that mixed a polars result into a pandas-typed local (`gfql/row/frame_ops.py`, `gfql_unified.py`) return or branch directly instead. Typing only: no runtime behaviour, no public API, no new dependency. Measured on a stub-visible mypy lane with polars importable, repo-wide errors go 163 -> 157 (11 -> 9 files); the lane CI actually runs today is unaffected and still reports no issues.
29
+
- **`rows(table=...)` and rewrite-param regression suites now run on all four engines** (tests only; zero runtime delta — no production line changed). The #1788 and #1790 suites were parametrized over pandas + polars only, so the `table` guards they exist to pin were never exercised on cuDF or polars-gpu at all. Both files were already in `bin/test-polars.sh` (#1805), so their polars params did run; what ran nowhere was `test_rows_table_named_middle.py` outside that one lane — a module-level `pytest.importorskip("polars")` skipped the whole file in `test-gfql-core`, which installs no polars. Both files are now parametrized over pandas / cuDF / polars / polars-gpu from a **fixed** engine list plus a classified availability check; dropping the module-level `importorskip` also makes 5 pandas cases live in `test-gfql-core`. The availability check is deliberately not `available_nonpandas_engines()`, which builds its list by importability so a missing engine vanishes from the report instead of showing as SKIPPED, and it is deliberately not a blanket `except Exception` either: a missing module skips, a recognisable GPU-stack error skips **with its text quoted**, and any other failure propagates — because a skipped GPU parameter reads as evidence of passing. Both traps were hit by trying rather than reasoned about: a probe that ran the shape under test turned a reverted production guard into 20 SKIPS instead of 20 failures, and a blanket probe silently dropped cuDF from an otherwise-green GPU run on a transient `cudaErrorMemoryAllocation`. GPU receipts on dgx GB10 / cuDF 26.02.01 / polars 1.35.2 / `docker run --gpus all`: **45 passed, 3 xfailed, 0 skipped**; mutation-checked by reverting the #1788 and #1790 `table` guards — **24 failures spread evenly over all four engines (6 each)**, so the added parameters are not decorative. Two gaps the new parameters surfaced are pinned as strict xfails rather than papered over: #1803 (the indexed bindings bypass excludes `Engine.POLARS_GPU`, so polars-gpu silently takes the scan path) and #1804 (the native polars bindings builder never receives `alias_prefilters` — previously an unnumbered xfail).
30
+
- **Row execution context: `clear_row_exec_context` NULLs rather than restores, and that is now pinned rather than assumed** (tests + comments only; zero runtime delta — no production line changed). Review of #1793 asked whether the exit should RESTORE what the graph carried on entry instead of nulling, since `attach_row_exec_context` INHERITS on the way in (a `None` argument keeps what `g` already carries). Settled as NULL, for three reasons now each carried by a test: (1) `clear` is pure — it returns `g.bind()` and never writes through to the object it was handed, so an outer scope that set the field still has it afterwards and there is no caller state to save; that is what separates this from #1786, which WAS an in-place write onto the caller's own graph, and restore is the fix for a mutation. (2) The only channel restore would change is the RETURN value, and putting the seed back there IS the second half of #1786 — measured, a seed hand-restored onto a result changes the answer of the next query run on that result (7 → 2 → 1 rows). (3) No execution frame inherits a context it did not set: instrumenting `attach` over `graphistry/tests/compute` recorded 3907 calls and zero inheriting ones (53 entered on a graph already carrying a seed, but each was handed the identical `start_nodes` parameter, so the frame still owns the value), because the cross-segment `WITH` seed travels as the explicit `start_nodes` PARAMETER and never through the graph field. The new `test_exec_context_scoping.py` re-runs that ownership measurement as an assertion, so a future path that starts relying on inheritance reopens the decision loudly instead of silently losing an outer value. Mutation-checked in the deciding direction: implementing save/restore at all three attach sites fails 4 of the new tests on every runnable engine, while the existing #1793 suite passes unchanged — i.e. those tests could not distinguish the two designs and this file can. Engine-parametrized over pandas/cuDF/polars/polars-gpu from a fixed list plus a classified availability check (a missing module skips, a recognisable GPU-stack error skips with its text quoted, and any other failure propagates — a silently skipped GPU parameter reads as evidence of passing); GPU receipts on dgx GB10 / cuDF 26.02.01 / `docker run --gpus all`: 43 passed, 0 skipped.
29
31
- **`is_lazy` is a type predicate, so the polars engine's eager/lazy split is checked instead of asserted**: `dtypes.is_lazy` decides WHICH member of the two-member `PolarsFrame` union a frame is, but it was declared `-> bool`, so that answer was discarded at the call boundary — an eager-only attribute (`.height`, `.columns`, `.schema`) reached after a lazy guard was unprovable, and the native polars chain closed the gap with `cast("pl.DataFrame", frame)`. A cast is not a type: it re-asserts, unchecked, the exact fact the predicate had just established, once per call site, and a wrong one surfaces as a production `AttributeError` rather than a CI error. `is_lazy` is now declared `-> TypeIs["pl.LazyFrame"]` (PEP 742), which narrows the *negative* branch as well as the positive — the eager side is the one that needed it, so `TypeGuard` would not have done — and the cast is gone. Four previously untyped chain-combine helpers (`_semi`, `_combine_edges`, `_apply_node_names`, and the `_known_empty` guard) now carry real signatures: `_semi`'s two frames share the `PolarsT` TypeVar because polars joins do not mix eagerness, and the two combine helpers take the `_LazyShim` collect-once duck-type they are actually called with. Typing `_apply_node_names` also retired a `getattr(next_step, "edges_empty", None)` — the shim declares that tri-state in `__slots__`, so a typo is now a checker error instead of a silent `None` that would have re-armed the very cardinality gate the guard disarms. Internal only: no runtime behaviour, no public API, and the `TypeIs` import is `TYPE_CHECKING`-only, so no new runtime dependency floor.
30
32
-**GFQL execution context is declared rather than attached at runtime**: the private `_gfql_*` per-execution fields (index policy and registry, row-pipeline base graph, carried seed nodes, edge aliases, shortest-path backend, and the indexed-bindings handoff) are now declared on `Plottable` with defaults on `PlotterBase`, instead of being set with `setattr` and read back with `getattr(..., default)`. No public API or behaviour change; internal call sites are typed, and hand-rolled `Plottable` stand-ins must now construct the full context.
31
33
32
34
### Fixed
35
+
- **Three BOUNDED variable-length shapes returned a silently different count on `engine='polars'` (#1787)**: the native polars `rows(binding_ops=...)` builder rebuilds a variable-length segment from the raw matching edge table, and for three bounded shapes that rebuild produces a different edge multiplicity than the pandas oracle — with no error, just a different number. They now raise `NotImplementedError` like every other shape this lowering cannot reproduce, which is a deliberate behaviour change: these were *served* before, so a silent wrong answer becomes a loud, actionable error, and `engine='pandas'` still answers all of them. The three: (1) directed `-[*k..m]->` with `min_hops >= 3`, and `min_hops >= 2` when the segment starts from a filtered seed — `max_reached_hop` in `compute/hop.py` is a dedup-by-node BFS eccentricity rather than a longest-walk length, so the oracle prunes to empty where the rebuild expands a different edge multiset; (2) the DEGENERATE undirected window `-[*1..1]-` / `-[*1]-`, which halved the count — it resolves to `min == max == 1` and so is not "multihop", yet pandas still routes it through the variable-length hop, which is exactly why it slipped past the existing gates (the gate therefore keys on an explicit variable-length window, not on `is_multihop`); (3) undirected `-[*1..k]-` that does not start from the full node set (filtered seed, or a non-first segment), where the doubled-pair expansion over-counts. Every neighbouring shape stays native and is pinned as such — unseeded `min_hops <= 2`, the directed twin of each undirected decline, the directed degenerate window, and the plain `-[]-` edge — so the gate is a scalpel rather than a blanket refusal of variable-length segments. Same root-cause family as the unbounded shapes #1781 declined; the gate should shrink again once the multiplicity is reconstructible. Found by differential fuzzing against the pandas oracle, and pinned by an engine-parametrized suite (pandas / polars / cuDF / polars-gpu) that encodes the intended per-engine behaviour — the polars engines must decline exactly where the pandas-API engines must answer — plus a seeded fuzz that fails if the gate declines too much.
33
36
- **`rows(table='edges')` silently returned the wrong table after a NAMED traversal**: the named-middle rewrite — which turns `[...named ops..., rows()]` into `rows(binding_ops=...)` so a Cypher multi-alias `RETURN` lowers to a bindings table — skipped itself when the call already carried `binding_ops`, `source` or `alias_endpoints`, but not when it carried a non-default `table`. So naming any op in the middle changed which table came back: an odd-length middle silently produced the BINDINGS table instead of the requested edges (no error, `table=` simply ignored), and an even-length middle — a path ending on an edge, e.g. `(person)-[r:KNOWS]-` — produced a non-alternating op list and raised "require ... a single connected alternating node/edge path". An unnamed middle was unaffected, which is what made it look shape-specific rather than naming-specific. Both chain surfaces are fixed (the generic chain and the native polars chain carry the rewrite independently, so fixing one left the other wrong). Note the guard keys on a NON-DEFAULT table: `rows()` declares `table='nodes'` and always emits it, so an explicit `rows(table='nodes')` is indistinguishable from a bare `rows()` and still rewrites — pinned as a known limitation rather than left to be rediscovered.
34
37
- **Indexed bindings bypass returned the WHOLE edge table for `rows(table='edges')`**: the bypass exists to SKIP the canonical traversal, so whatever it hands the suffix is the pre-traversal graph. That is sound for a bindings table — the indexed path bag already is the answer — but the gate declined only for `source` / `alias_endpoints` / `alias_prefilters`, not for a non-default `table`, so a `rows(table='edges')` after a named middle read the full edge frame instead of the traversal-narrowed one (measured on a 12-edge fixture: 12 rows instead of 3, on pandas, cuDF and native polars, and the wrong values survive a following `select`). Both gates now decline a non-default `table` (`chain._plan_indexed_middle` and the native polars `_try_indexed_middle_polars`), matching the named-middle rewrite's guard. This became reachable only once that rewrite stopped firing for a non-default `table` — before it, the rewrite converted the call on BOTH the indexed and the scan path, so the two agreed on the wrong table. Same class as the earlier "indexed bypass could return every node for `rows()` over an unnamed pattern".
35
38
- **The named-middle rewrite discarded the caller's other `rows()` params**: both rewrites built a FRESH `rows(binding_ops=...)` instead of adding `binding_ops` to the call the caller wrote, so every param the rewrite has no opinion about was thrown away. For `attach_prop_aliases` — the projection pushdown — that is observable: spelling a pattern as a NAMED middle rather than as explicit `binding_ops` silently attached every alias's properties instead of the requested ones, i.e. the same query returned a wider schema depending only on how it was written. `alias_prefilters` was dropped the same way. Both are now carried through, on both chain surfaces. Known remaining gap, pinned by a strict-xfail test rather than left to be rediscovered: the native polars bindings builder never receives `alias_prefilters` at all, so hand-written GFQL passing that hint without an equivalent post-filter still gets engine-dependent row counts (Cypher-generated plans always keep the post-filter, so they agree across engines and only lose the pushdown).
0 commit comments