diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f844842c8..46d0bee136 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - **GFQL Cypher parser: Earley → LALR(1) (~70× faster parse, same AST)**: `parse_cypher` now uses a single LALR(1) parser (contextual lexer) instead of Earley (dynamic lexer) — ~0.25ms vs ~17ms per query on representative queries. The WHERE grammar was unified to one `where_clause: "WHERE" expr` rule (the former `where_predicates | expr` dual rule was a genuine reduce/reduce ambiguity that forced Earley), and the structured flat-AND predicate form is recovered by a post-parse lift that re-parses the WHERE body with a LALR sub-parser (`start="where_predicate_chain"`). The full WHERE language is preserved — OR/XOR/NOT, parentheses, arithmetic, IN, pattern predicates — nothing is restricted to a subset. The grammar was then **purified to (near-)unambiguous**: WHERE binds to its preceding clause *in the grammar* (bundled into `match_clause`; the WITH..WHERE attachment ambiguity is eliminated, not tie-broken), and name-rooted dot chains derive only via `qualified_name` (the `property_access` redundancy is gone) — dropping the redundant `label_predicate_expr` rule (`(n:Admin)` parses via `grouped_expr`), and excluding a top-level `IN` from list-literal elements (`[x IN xs ...` is comprehension syntax; allowing a bare `IN` list element made it overlap). Net: the LALR conflict profile goes from 8 shift/reduce to **ZERO conflicts** — the grammar is now **provably unambiguous LALR(1)** and builds under Lark's `strict=True` (a build-time proof, machine-checked in CI as zero conflicts, plus a strict-mode build test where the optional `interegular` dep is present). Every input has a single derivation. **Machine-checked invariants** (`test_grammar_invariants.py`): (1) **zero LALR conflicts** (dependency-free, always-on in CI) plus a `strict=True` build test (skipped where the optional `interegular` dep is absent) — a grammar edit that introduces any ambiguity fails CI; (2) semantic ambiguity is ZERO — every corpus query has exactly one Earley derivation and it transforms to a single AST, no exceptions; (3) a rule-coverage gate (`test_every_grammar_rule_is_exercised_by_the_corpus`) forces every new grammar rule through the invariants; (4) differential tests run the production pipeline under both parsers (byte-identical ASTs, identical rejections). **Full-repo differentials** (1,850+ scraped queries, LALR-vs-Earley and old-vs-new production): **zero AST divergences**, and a small set of deliberate language fixes — accept-by-accident shapes with ill-defined semantics, now honest syntax errors and pinned as tests (`DELIBERATE_LANGUAGE_FIXES`): `RETURN DISTINCT` with no items (Earley re-lexed DISTINCT as a variable name), double WHERE (the old positional attachment kept *both* predicates in different AST fields), double post-WITH WHERE, WHERE after UNWIND, WHERE before MATCH in a graph constructor, and the invalid "list of IN-booleans" (`[x IN xs, y]`; use `[(x IN xs), y]` for a genuine bool list). The lift stays an internal optimization: only a flat `AND` chain over present columns lifts to `filter_dict`; parens / OR / XOR / NOT and any absent-column case stay on `where_rows` (a correctness boundary, since `where_rows` treats an absent property as null while `filter_dict` would raise). Full cypher suite (1,681 tests) passes. Downstream execution (`filter_dict` vs `where_rows` routing) is unchanged. A pre-existing `<>`-over-null 3VL divergence between the two execution paths (surfaced by #1653's metamorphic test) is tracked separately in #1683. ### Fixed +- **GFQL seeded indexes now engage for small Polars/cuDF frontiers**: NaN-free native Polars frames retain object identity during normalization, preserving resident CSR validity, while a tunable absolute floor prevents the fractional cost gate from routing tiny seeded hops over low-cardinality slices to a full scan. Scan/index result parity is unchanged; the default floor is a routing heuristic whose engine/data crossover remains subject to measurement. - **GFQL Cypher `GRAPH { }` residual predicates now fail safely or apply as graph masks**: `GRAPH { MATCH ... WHERE ... }` no longer silently drops predicates that the graph-state path cannot apply. Safe one-node/one-edge residual filters, including disjunctions and `searchAny(...)`, are applied before graph-state matching; unsupported pattern-predicate, multi-alias, and Polars graph-residual cases now raise clear validation errors instead of returning an over-broad subgraph. - **GFQL Cypher `=~` / scalar-fn cross-engine hardening (review wave, dgx-verified)**: (1) composed `=~` (`WHERE … =~ … OR …`, `RETURN`-expression position) now works on `engine='cudf'` — the series evaluator used raw `.str.fullmatch`, which cuDF lacks, and the resulting `AttributeError` was masked as "unsupported predicate op" (it now routes through the `Fullmatch` predicate's engine workarounds, and honest `NotImplementedError` declines pass through instead of being re-labeled); (2) the cuDF fullmatch emulation anchors alternations as a whole (`^(ab|cd)$` — bare `^ab|cd$` silently matched `'abXXX'`); (3) the cuDF case-insensitive `(?i)` lowercase-folding workaround now declines the fold-unsound shapes — uppercase escape classes (`.lower()` turns `\D` into `\d`, silently inverting the predicate), case-crossing character ranges (`(?i)[A-z]` silently narrowed; `[X-b]` folded to an invalid range), and non-ASCII patterns — while lowercase escapes (`\d`, `\.`) keep folding as before; lookaround, backreferences, and named-group refs decline up front (libcudf rejects them at kernel-compile time); (4) polars `Match`/`Fullmatch` lowering applies the same Rust-regex guard as `Contains` (lookaround/backrefs decline instead of a non-NIE `ComputeError` at collect); (5) `toLower`/`toUpper` on a non-string column decline like neo4j's type error instead of broadcasting the stringified Series repr (pandas/cuDF) or raising a non-NIE `SchemaError` (polars-gpu); (6) polars `floor`/`ceil`/`round` cast to `Float64` so integer columns return Float like neo4j and the pandas engine; (7) invalid regex patterns on the composed path raise a clear "invalid regex pattern" error instead of "unsupported predicate op". - **GFQL Cypher `round()` hardening (review wave, dgx-verified)**: (1) the `polars` extra's floor is now `polars>=1.29` — `Expr.round(mode=)` shipped in py-1.29.0 (pola-rs/polars#22248), not 1.5 as previously pinned, so 1.5–1.28 installs crashed with a raw `TypeError` on `round(x, p>0)` under `engine='polars'`; (2) `round(x, p>308)` is the identity on both engines (a float64 has no digits there) instead of pandas raising through an unclear decline while polars returned identity — parity restored, `10.0**p` overflow guarded; (3) polars `round(x, p>0)` normalizes `-0.0` to `+0.0` like the pandas kernel (`round(-0.04, 1)` was `0.0` on pandas vs `-0.0` on polars — invisible to value equality, pinned by a sign-bit test); (4) documented the precision>0 decimal-string deviation vs neo4j (`round(2.675, 2)` = `2.67` binary-double here vs `2.68` BigDecimal there) and added deterministic tie/hazard matrix cases so ties actually reach cuDF/polars-gpu (the fixture's normal-distribution floats never tied). diff --git a/bin/test-polars.sh b/bin/test-polars.sh index ae429df75d..6d7965cee6 100755 --- a/bin/test-polars.sh +++ b/bin/test-polars.sh @@ -3,7 +3,7 @@ set -ex # Run from project root # - Extra args are passed through to the pytest phase -# - Set POLARS_COV=1 to collect coverage over graphistry/compute; the coverage +# - Set POLARS_COV=1 to collect coverage over the graphistry package; the coverage # data file location is taken from $COVERAGE_FILE (as the CI py3.12 lane sets it) # - Non-zero exit code on fail @@ -23,11 +23,19 @@ POLARS_TEST_FILES=( # index tests exercise the seeded-index hook in the polars hop entry (hop.py) — without # them the hook dominates the now-thin file and trips its per-file coverage floor graphistry/tests/compute/gfql/index/test_index.py + # Engine coercion tests: the polars paths (df_to_engine, _pl_nan_to_null identity) only + # run where polars is installed, and this lane is the only coverage-collecting lane + # with polars — the core py3.14 lane has no polars extra, so those tests skip there + graphistry/tests/compute/test_engine_coercion.py ) +# The whole graphistry package is measured here (not just graphistry/compute) because +# polars-only branches outside compute/ (e.g. Engine._pl_nan_to_null) are unreachable in +# the core coverage lane (no polars extra); coverage sources must be dirs/packages, and a +# dotted module source (graphistry.Engine) breaks numpy under pytest COV_ARGS=() if [ -n "${POLARS_COV:-}" ]; then - COV_ARGS=(--cov=graphistry/compute --cov-report=) + COV_ARGS=(--cov=graphistry --cov-report=) fi python -B -m pytest -vv "${COV_ARGS[@]}" "${POLARS_TEST_FILES[@]}" "$@" @@ -36,7 +44,7 @@ python -B -m pytest -vv "${COV_ARGS[@]}" "${POLARS_TEST_FILES[@]}" "$@" # appended into the same coverage data file when POLARS_COV=1 (CI audit reads it) COV_APPEND_ARGS=() if [ -n "${POLARS_COV:-}" ]; then - COV_APPEND_ARGS=(--cov=graphistry/compute --cov-report= --cov-append) + COV_APPEND_ARGS=(--cov=graphistry --cov-report= --cov-append) fi python -B -m pytest -vv "${COV_APPEND_ARGS[@]}" \ graphistry/tests/compute/gfql/cypher/test_lowering.py -k polars diff --git a/graphistry/Engine.py b/graphistry/Engine.py index a3d93c2939..a19c8f1449 100644 --- a/graphistry/Engine.py +++ b/graphistry/Engine.py @@ -214,11 +214,25 @@ def _pl_nan_to_null(df): polars / Arrow / cuDF input carrying genuine NaN is treated as MISSING like the pandas oracle (which skipna/dropna's NaN). Without this, ``engine='polars'`` on a frame with a real NaN keeps rows a filter/aggregation should drop (silent divergence from pandas). - No-op when there are no float columns.""" + No-op when there are no float columns. + + Identity-stable: returns the *same* frame object when no float column actually carries a + NaN. ``fill_nan(None)`` on a NaN-free column is a value-level no-op but still yields a + FRESH frame; that identity churn defeats frame-identity caches keyed on ``source_ref is + df`` (the #1658 CSR index) -- see #1726. So we probe for real NaN presence per float + column (``is_nan`` only applies to float dtypes, hence the schema gate) and rebuild only + when -- and only the columns where -- a NaN is genuinely present. Values are identical to + the old unconditional rewrite (``fill_nan(None)`` never touches non-NaN entries).""" import polars as pl float_cols = [c for c, dt in df.schema.items() if dt in (pl.Float32, pl.Float64)] if not float_cols: return df + if isinstance(df, pl.DataFrame): + nan_cols = [c for c in float_cols if df.get_column(c).is_nan().any()] + if not nan_cols: + return df + return df.with_columns([pl.col(c).fill_nan(None) for c in nan_cols]) + # LazyFrame (rare): no cheap eager NaN probe, keep the original unconditional rewrite. return df.with_columns([pl.col(c).fill_nan(None) for c in float_cols]) diff --git a/graphistry/compute/gfql/index/__init__.py b/graphistry/compute/gfql/index/__init__.py index 7ee6cbf4f8..f58588bb79 100644 --- a/graphistry/compute/gfql/index/__init__.py +++ b/graphistry/compute/gfql/index/__init__.py @@ -23,7 +23,9 @@ is_index_op, is_index_op_json, ) from .cypher_ddl import parse_index_ddl, looks_like_index_ddl -from .cost import cost_gate_frac, reset_cost_gate_frac, set_cost_gate_frac +from .cost import ( + cost_gate_frac, cost_gate_min_frontier, reset_cost_gate_frac, set_cost_gate_frac, +) from .explain import GfqlExplainReport __all__ = [ @@ -38,6 +40,7 @@ "CreateIndex", "DropIndex", "ShowIndexes", "IndexOp", "apply_index_op", "index_op_from_json", "is_index_op", "is_index_op_json", "parse_index_ddl", "looks_like_index_ddl", - "cost_gate_frac", "reset_cost_gate_frac", "set_cost_gate_frac", + "cost_gate_frac", "cost_gate_min_frontier", "reset_cost_gate_frac", + "set_cost_gate_frac", "GfqlExplainReport", ] diff --git a/graphistry/compute/gfql/index/api.py b/graphistry/compute/gfql/index/api.py index bc6f751e5a..f5296bd911 100644 --- a/graphistry/compute/gfql/index/api.py +++ b/graphistry/compute/gfql/index/api.py @@ -21,7 +21,7 @@ ) from .build import build_adjacency_index, build_node_id_index from .traverse import index_seeded_hop -from .cost import cost_gate_frac, seed_deg_sum, seed_id_array +from .cost import cost_gate_frac, cost_gate_min_frontier, seed_deg_sum, seed_id_array from .policy import IndexPolicy, validate_index_policy from .types import ( AdjacencyIndexKind, EdgeIndexDirection, HopDirection, IndexKind, @@ -424,10 +424,15 @@ def _bail(reason: str) -> Optional[Plottable]: # Cost gate: if the frontier covers a large fraction of distinct sources, the # scan path is competitive — fall back (avoids index overhead on bulk-ish hops). + # Small-frontier floor: a frontier of <= cost_gate_min_frontier() seeds always + # indexes — the frac gate scales with n_keys, so on small/low-cardinality slices + # it would otherwise send even a single-seed hop to the O(E) scan. Pure routing + # heuristic either way: index and scan return identical results. idx0 = cast(Optional[AdjacencyIndex], registry.get_valid( EDGE_OUT_ADJ if direction != "reverse" else EDGE_IN_ADJ, g._edges, (src, dst), engine )) frac = cost_gate_frac(engine) + min_frontier = cost_gate_min_frontier() if trace and idx0 is not None: # Free fanout estimate (Σ seed degree) from the CSR offsets — the planner # signal the report wants EXPLAIN to surface (not just used-index yes/no). @@ -437,16 +442,19 @@ def _bail(reason: str) -> Optional[Plottable]: diag["seed_deg_sum"] = deg_sum diag["est_result_rows"] = deg_sum diag["threshold_frac"] = frac + diag["min_frontier_floor"] = min_frontier if idx0 is None: # required direction not resident (undirected needs both); let driver decide pass elif resolved_policy != "force": try: frontier_n = int(nodes.shape[0]) - if idx0.n_keys > 0 and frontier_n >= frac * idx0.n_keys: + if (frontier_n > min_frontier + and idx0.n_keys > 0 and frontier_n >= frac * idx0.n_keys): return _bail( f"frontier {frontier_n} >= {frac}*n_keys " - f"({frac * idx0.n_keys:.0f}) -> scan cheaper" + f"({frac * idx0.n_keys:.0f}) and > floor ({min_frontier}) " + f"-> scan cheaper" ) except (AttributeError, TypeError, ValueError): pass diff --git a/graphistry/compute/gfql/index/cost.py b/graphistry/compute/gfql/index/cost.py index 754db74482..bd8ca53862 100644 --- a/graphistry/compute/gfql/index/cost.py +++ b/graphistry/compute/gfql/index/cost.py @@ -17,6 +17,46 @@ _COST_GATE_FRAC_DEFAULT = 0.02 _COST_GATE_FRAC_OVERRIDES: Dict[Engine, float] = {} +# Absolute small-frontier floor: at or below this many seed rows the planner NEVER +# bails to scan on the frac gate. The frac gate scales with distinct-key cardinality, +# so on small / low-cardinality edge slices (e.g. per-edge-type homogeneous frames: +# n_keys <= 1/0.02 = 50 at the polars/cudf frac) even a single-node seed trips it and +# the hop scans O(E) despite a resident O(degree) index. A frontier of <= K seeds +# bounds CSR lookup work to K searchsorted probes plus the matching-row gather, +# avoiding that fractional-gate artifact. The actual index/scan crossover remains +# engine- and data-dependent: K is a conservative, environment-tunable default, +# not a measured universal threshold. Constant (not a function of n_keys) on +# purpose: scaling with n_keys is the frac gate's job; the floor bounds absolute +# per-hop probe work where frac*n_keys collapses below a handful of seeds. Uniform +# across engines (pandas's 0.5 frac only overlaps the floor when n_keys <= 2*K). +# Purely a routing heuristic: index and scan return identical results. +_COST_GATE_MIN_FRONTIER_DEFAULT = 16 + + +def cost_gate_min_frontier() -> int: + """Return the absolute small-frontier floor for the index-vs-scan cost gate. + + Frontiers of at most this many seeds always take a resident index, regardless + of the frac gate. ``0`` disables the floor (pure frac gating). Env-overridable + via ``GFQL_INDEX_COST_GATE_MIN_FRONTIER`` for benchmark/diagnostic tuning. + """ + raw = os.environ.get("GFQL_INDEX_COST_GATE_MIN_FRONTIER") + if raw is None or raw == "": + return _COST_GATE_MIN_FRONTIER_DEFAULT + try: + val = int(raw) + except ValueError as ex: + raise ValueError( + f"Invalid GFQL_INDEX_COST_GATE_MIN_FRONTIER={raw!r}: " + f"expected a non-negative integer" + ) from ex + if val < 0: + raise ValueError( + f"Invalid GFQL_INDEX_COST_GATE_MIN_FRONTIER={raw!r}: " + f"expected a non-negative integer" + ) + return val + def _validate_cost_gate_frac(frac: float) -> float: if not 0.0 < frac <= 1.0: diff --git a/graphistry/compute/gfql/index/types.py b/graphistry/compute/gfql/index/types.py index af3aa800f6..47a088b73c 100644 --- a/graphistry/compute/gfql/index/types.py +++ b/graphistry/compute/gfql/index/types.py @@ -30,6 +30,7 @@ class IndexTraceStep(TypedDict, total=False): seed_deg_sum: Optional[int] est_result_rows: Optional[int] threshold_frac: float + min_frontier_floor: int IndexTrace = List[IndexTraceStep] diff --git a/graphistry/tests/compute/gfql/index/test_index.py b/graphistry/tests/compute/gfql/index/test_index.py index c8a48bf216..a48dbb97b1 100644 --- a/graphistry/tests/compute/gfql/index/test_index.py +++ b/graphistry/tests/compute/gfql/index/test_index.py @@ -197,6 +197,8 @@ def test_explain_exposes_planner_diagnostics(graph, engine): free Σ-degree fanout estimate (from CSR group_offsets), chosen direction, the cost-gate threshold, and a human-readable decision_reason — not just a used-index yes/no. This is the EXPLAIN enrichment the indexing roadmap calls for.""" + from graphistry.compute.gfql.index import cost_gate_min_frontier + chain = [n({"id": 0}), e_forward(hops=1)] # force → index path taken → per-step + top-level diagnostics populated rep = graph.gfql_explain(chain, index_policy="force", engine=engine) @@ -205,10 +207,14 @@ def test_explain_exposes_planner_diagnostics(graph, engine): assert isinstance(rep["est_result_rows"], int) and rep["est_result_rows"] >= 0 assert "index" in (rep["decision_reason"] or ""), rep["decision_reason"] step = next(s for s in rep["steps"] if s.get("path") == "index") - for k in ("frontier_n", "n_keys", "seed_deg_sum", "est_result_rows", "threshold_frac"): + for k in ( + "frontier_n", "n_keys", "seed_deg_sum", "est_result_rows", + "threshold_frac", "min_frontier_floor", + ): assert k in step, (k, step) assert step["est_result_rows"] == step["seed_deg_sum"] # est == free Σ-degree assert step["seed_deg_sum"] >= 0 + assert step["min_frontier_floor"] == cost_gate_min_frontier() # off (index resident) → the planner records *why* it scanned, not just that it did gi = graph.gfql_index_all(engine=engine) @@ -324,6 +330,89 @@ def test_cost_gate_frac_tuning(monkeypatch): reset_cost_gate_frac() +@pytest.mark.parametrize("engine", ENGINES) +def test_cost_gate_small_frontier_floor_indexes_tiny_slices(engine): + """#1726 follow-up: a small seeded frontier on a small-n_keys slice (e.g. an SNB + per-edge-type homogeneous frame) must take the resident index on every engine. + Before the floor, the frac gate (0.02 on polars/cudf) tripped on any slice with + n_keys <= 50 even for a single seed -> O(E) scan with an O(degree) index resident. + Result is identical either way (the gate is a pure routing heuristic).""" + from graphistry.compute.gfql.index import index_trace + rng = np.random.default_rng(3) + n_src = 30 # n_keys ~30: frac gate alone would demand frontier < 0.6 on polars/cudf + edf = pd.DataFrame({ + "src": rng.integers(0, n_src, 600), + "dst": rng.integers(0, 2000, 600), + }) + g = graphistry.edges(edf, "src", "dst").materialize_nodes() + gi = g.gfql_index_all(engine=engine) + seeds = pd.DataFrame({"id": edf["src"].iloc[:1].astype("int64")}) # frontier_n=1 + with index_trace() as steps: + out = gi.hop(nodes=seeds, engine=engine, hops=1, direction="forward") + assert any(s.get("path") == "index" for s in steps), (engine, steps) + scan_out = g.hop(nodes=seeds, engine=engine, hops=1, direction="forward") + assert _sig(out) == _sig(scan_out), engine # correctness path-independent + + # A frontier just past the floor on the same tiny slice still bails to scan + # (frac gate: 17 >= 0.02*30 and 17 >= 0.5*30) -> the bulk-hop protection stands. + many = pd.DataFrame({"id": np.arange(17, dtype="int64")}) + with index_trace() as steps2: + out2 = gi.hop(nodes=many, engine=engine, hops=1, direction="forward") + assert not any(s.get("path") == "index" for s in steps2), (engine, steps2) + assert any("scan cheaper" in (s.get("decision_reason") or "") for s in steps2), (engine, steps2) + scan_out2 = g.hop(nodes=many, engine=engine, hops=1, direction="forward") + assert _sig(out2) == _sig(scan_out2), engine + + +def test_cost_gate_min_frontier_tuning(monkeypatch): + from graphistry.compute.gfql.index import cost_gate_min_frontier + from graphistry.compute.gfql.index.cost import _COST_GATE_MIN_FRONTIER_DEFAULT + + monkeypatch.delenv("GFQL_INDEX_COST_GATE_MIN_FRONTIER", raising=False) + assert cost_gate_min_frontier() == _COST_GATE_MIN_FRONTIER_DEFAULT == 16 + + monkeypatch.setenv("GFQL_INDEX_COST_GATE_MIN_FRONTIER", "32") + assert cost_gate_min_frontier() == 32 + + monkeypatch.setenv("GFQL_INDEX_COST_GATE_MIN_FRONTIER", "0") # 0 disables the floor + assert cost_gate_min_frontier() == 0 + + monkeypatch.setenv("GFQL_INDEX_COST_GATE_MIN_FRONTIER", "-1") + with pytest.raises(ValueError, match="GFQL_INDEX_COST_GATE_MIN_FRONTIER"): + cost_gate_min_frontier() + monkeypatch.setenv("GFQL_INDEX_COST_GATE_MIN_FRONTIER", "abc") + with pytest.raises(ValueError, match="GFQL_INDEX_COST_GATE_MIN_FRONTIER"): + cost_gate_min_frontier() + + +def test_cost_gate_min_frontier_floor_disabled_restores_frac_gate(monkeypatch): + """Floor=0 restores pure frac gating: a 1-seed hop on a tiny-n_keys slice bails + again under a sub-frac (proves the floor is what routes it to the index).""" + from graphistry.Engine import Engine + from graphistry.compute.gfql.index import ( + index_trace, reset_cost_gate_frac, set_cost_gate_frac, + ) + monkeypatch.delenv("GFQL_INDEX_COST_GATE_MIN_FRONTIER", raising=False) + edf = pd.DataFrame({"src": [0, 1, 2, 0, 1], "dst": [10, 11, 12, 13, 14]}) + g = graphistry.edges(edf, "src", "dst").materialize_nodes() + gi = g.gfql_index_all(engine="pandas") + seeds = pd.DataFrame({"id": [0]}) + set_cost_gate_frac(Engine.PANDAS, 0.02) # emulate the vectorized-engine frac on CPU + try: + # frac alone would bail (1 >= 0.02*3): the floor keeps the index engaged + with index_trace() as steps: + gi.hop(nodes=seeds, engine="pandas", hops=1, direction="forward") + assert any(s.get("path") == "index" for s in steps), steps + # disable the floor -> the frac gate governs again -> scan + monkeypatch.setenv("GFQL_INDEX_COST_GATE_MIN_FRONTIER", "0") + with index_trace() as steps2: + gi.hop(nodes=seeds, engine="pandas", hops=1, direction="forward") + assert not any(s.get("path") == "index" for s in steps2), steps2 + assert any("scan cheaper" in (s.get("decision_reason") or "") for s in steps2), steps2 + finally: + reset_cost_gate_frac() + + def test_column_mismatch_raises_not_silent(graph): # A custom column that doesn't match the binding must raise, not silently no-op. with pytest.raises(NotImplementedError): diff --git a/graphistry/tests/compute/test_engine_coercion.py b/graphistry/tests/compute/test_engine_coercion.py index 574900673e..853591603b 100644 --- a/graphistry/tests/compute/test_engine_coercion.py +++ b/graphistry/tests/compute/test_engine_coercion.py @@ -356,6 +356,48 @@ def test_polars_idempotent(self): self.assertIs(twice._edges, once._edges) +class TestPlNanToNull(NoAuthTestCase): + """_pl_nan_to_null must be identity-stable when no NaN is present (#1726) while + preserving the NaN -> null conversion when a float column actually carries a NaN.""" + + @unittest.skipUnless(HAS_POLARS, "polars not installed") + def test_float_col_without_nan_returns_same_object(self): + # A NaN-free float column must NOT trigger a rebuild (frame-identity churn would + # defeat the #1658 CSR index, which keys resident indexes on `source_ref is df`). + from graphistry.Engine import _pl_nan_to_null + inp = pl.DataFrame({"src": ["a", "b"], "w": [1.0, 2.0]}) + out = _pl_nan_to_null(inp) + self.assertIs(out, inp) + + @unittest.skipUnless(HAS_POLARS, "polars not installed") + def test_no_float_cols_returns_same_object(self): + from graphistry.Engine import _pl_nan_to_null + inp = pl.DataFrame({"src": ["a", "b"], "dst": ["b", "c"]}) + out = _pl_nan_to_null(inp) + self.assertIs(out, inp) + + @unittest.skipUnless(HAS_POLARS, "polars not installed") + def test_float_col_with_nan_still_converts_to_null(self): + # NaN-present path is unchanged: NaN -> null, other values untouched. + from graphistry.Engine import _pl_nan_to_null + inp = pl.DataFrame({"src": ["a", "b", "c"], "w": [1.0, float("nan"), 3.0]}) + out = _pl_nan_to_null(inp) + self.assertIsNot(out, inp) + w = out.get_column("w") + # The NaN became a genuine null (not still a NaN), non-NaN values preserved. + self.assertEqual(w.null_count(), 1) + self.assertFalse(bool(w.is_nan().any())) + self.assertEqual(w.to_list(), [1.0, None, 3.0]) + + @unittest.skipUnless(HAS_POLARS, "polars not installed") + def test_existing_nulls_without_nan_returns_same_object(self): + # A float column with real nulls but no NaN needs no rewrite (identity preserved). + from graphistry.Engine import _pl_nan_to_null + inp = pl.DataFrame({"src": ["a", "b"], "w": [1.0, None]}) + out = _pl_nan_to_null(inp) + self.assertIs(out, inp) + + class TestToPandas(NoAuthTestCase): """to_pandas() method — converts all supported input types to pandas."""