fix(gfql): restore identity-stable _pl_nan_to_null (regression from #1731)

CHANGELOG.md

@@ -12,6 +12,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - **GFQL engine-selection docs (pandas / polars / cuDF / polars-gpu)**: New :doc:`Choosing a GFQL Engine <gfql/engines>` page — a numbers-first, persona-tested guide to the four interchangeable engines. Adds the one-keyword `engine='polars'` speedup (up to ~38× over pandas on real graphs, no GPU), a motivating warm-median comparison table on real public graphs (LiveJournal 35M / Orkut 117M), a decision matrix (workload shape × size × hardware → engine, with the measured ~10K-edge CPU crossover, GPU-work-bound rule, polars-gpu memory-pressure caveat, and GPU-or-error contract), a cuDF-vs-polars-gpu disambiguation (eager-op vs fused-lazy; cuDF is not deprecated), an honest "when *not* to use Polars" section, the differential-parity guarantee, and a methodology + reproducer-script disclosure. Rewrote the top of `gfql/performance.rst` to lead with the engine comparison (de-marketed the prose), wired the new page into the GFQL toctree + recommended paths, and added Polars/polars-gpu to the engine examples in `gfql/quick.rst` and `gfql/about.rst` (previously only pandas/cuDF were documented). Driven by 4-persona doc user-testing (pandas DS, RAPIDS user, perf engineer, skeptical evaluator).
 
 ### Fixed
+- **GFQL `#1658` seeded index re-engages on Polars/Polars-GPU for float-column graphs (restores identity-stable `_pl_nan_to_null`, regression from #1731)**: #1726 made `Engine._pl_nan_to_null` identity-stable — it probes `is_nan().any()` per float column and returns the *same* frame object when no NaN is present, because `fill_nan(None)` on a NaN-free column is a value-level no-op that still yields a fresh frame, and that identity churn defeats the #1658 CSR index's `source_ref is df` residency check (so Polars/Polars-GPU seeded hops silently fell back to the O(E) scan). PR #1731's "final hardening" (commit `c2138b0d`) accidentally reverted this to the unconditional `df.with_columns([fill_nan(None) ...])`, re-breaking index engagement on any graph carrying a Float32/Float64 column (SNB/LDBC, graph-bench); all-int/str graphs like Pokec were unaffected via the no-float-columns fast return. This restores the identity-stable probe (rebuilding only — and only the columns where — a NaN is genuinely present; the rare LazyFrame path keeps the unconditional rewrite). Value-identical to the old behavior; only object identity changes so the resident index survives a polars conversion. Regression guard `TestPlNanToNull` in `test_engine_coercion.py` (restored + kept as a regression fence).
 - **GFQL Cypher `CASE` with mixed-dtype branches is now engine-consistent on cuDF (no more `GFQLTypeError`)**: pandas coerces the two `CASE` branches to a common type, but cuDF's `.where` raised `TypeError: cudf does not support mixed types`, surfaced as a hard `GFQLTypeError`. This bit `CASE WHEN path IS NULL THEN -1 ELSE length(path) END` over an UNREACHABLE `shortestPath` (int `-1` branch vs an object/null hops branch) — pandas returned `-1`, cuDF errored. The row-AST `CASE` evaluator now unifies the branch dtypes and retries, so cuDF returns the same value pandas does. Regression test `test_case_mixed_dtype.py`.
 - **GFQL polars engine raises a clean typed error (not an opaque polars `InvalidOperationError`) for a string predicate on a non-string column**: `WHERE col STARTS WITH/CONTAINS/regex ...` on a Categorical/Enum/numeric column raised a clean `GFQLSchemaError` ("string predicate used on non-string column") on pandas/cuDF but leaked polars' internal `InvalidOperationError: expected String type, got: cat` on polars. `filter_by_dict_polars` now raises the same `GFQLSchemaError` (categorical treated as non-string, exactly as `filter_by_dict`), so all three engines agree. Regression test `test_polars_string_predicate_nonstring.py`; also confirms connected-join predicate pushdown is parity-correct on polars for string/numeric/eq/bool columns.
 - **GFQL string predicates (`Contains`/`Startswith`/`Endswith`/`Match`/`Fullmatch`) are now value-safe on non-string columns**: applying a string predicate to a numeric/temporal/bool column raised an opaque `AttributeError: Can only use .str accessor with string values!` on pandas and cuDF. They now follow openCypher semantics — a string op over a non-string value is null → excluded (matching the established per-cell behavior on an object column holding non-strings) — and never stringify the column (which would diverge pandas↔cuDF, e.g. wrongly matching `5 CONTAINS '5'`). String, mixed-object, and all-null columns are unchanged. Regression tests in `test_str.py` across dtypes; cuDF parity confirmed.

graphistry/Engine.py

@@ -214,11 +214,30 @@ 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).
+
+    NOTE: this identity guard was originally landed in #1726 (3809ecd9) and then accidentally
+    reverted by #1731 (c2138b0d, "final hardening"); this restores it. Regression bit polars/
+    polars-gpu index engagement on any float-column graph (SNB/LDBC, graph-bench); Pokec was
+    unaffected because its nodes are all int/str (the no-float-columns fast return)."""
     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])
 
 

graphistry/tests/compute/test_engine_coercion.py

@@ -684,5 +684,73 @@ def test_chain_dask_edges(self):
         self.assertIsInstance(result._edges, pd.DataFrame)
 
 
+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.
+
+    Regression guard: #1726 (3809ecd9) added this; #1731 (c2138b0d) reverted it; restored here.
+    """
+
+    @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)
+
+    @unittest.skipUnless(HAS_POLARS, "polars not installed")
+    def test_multi_float_col_rebuilds_only_nan_carrying_columns(self):
+        # With several float columns, only the ones that actually carry a NaN are converted;
+        # NaN-free float columns are left byte-for-byte as-is (values identical to the old
+        # unconditional rewrite, which is a no-op on non-NaN entries anyway).
+        from graphistry.Engine import _pl_nan_to_null
+        inp = pl.DataFrame({
+            "id": [0, 1, 2],
+            "clean": [1.0, 2.0, 3.0],                 # float, no NaN -> untouched
+            "dirty": [1.0, float("nan"), 3.0],        # float, has NaN -> converted
+        })
+        out = _pl_nan_to_null(inp)
+        self.assertIsNot(out, inp)                     # a NaN was present -> new frame
+        # NaN-free column unchanged (still float, no nulls introduced).
+        self.assertEqual(out.get_column("clean").to_list(), [1.0, 2.0, 3.0])
+        self.assertEqual(out.get_column("clean").null_count(), 0)
+        # NaN column converted to null, non-NaN values preserved.
+        self.assertEqual(out.get_column("dirty").to_list(), [1.0, None, 3.0])
+        self.assertEqual(out.get_column("dirty").null_count(), 1)
+        # Column order + non-float columns preserved.
+        self.assertEqual(out.columns, ["id", "clean", "dirty"])
+        self.assertEqual(out.get_column("id").to_list(), [0, 1, 2])
+
+
 if __name__ == "__main__":
     unittest.main()