Skip to content

Commit 458498f

Browse files
lmeyerovclaude
andcommitted
test(gfql): pin the pandas-3 case-mapping divergence the new lane coverage found (#1812)
Putting `test_viz_pipeline_conformance.py` into the polars lane turned it red on the first CI run — which is the point of the parent commit, and the finding is real, not a fixture problem. The polars lane resolves **pandas 3.0.3**; local dev and `test-gfql-core` resolve 2.3.3. pandas 3 defaults to a PyArrow-backed `str` dtype whose `utf8_upper`/`utf8_lower` do SIMPLE Unicode case mapping, where Python, polars and pandas 2 do FULL mapping. Measured: Python polars pandas 2 pandas 3 'ss'.upper() 'SS' 'SS' 'SS' U+1E9E 'I-dot'.lower() 'i'+U+0307 same same 'i' So `MATCH (n) WHERE toUpper(n.name) = 'STRASSE'` returns a different row set on pandas 3 than on polars, with NO error — and pandas is the oracle the parity suites compare against. Filed as #1812; it needs an owner call on which mapping is correct (Neo4j uses Java's full mapping; openCypher does not pin it), so it is not fixed here. Pinned with `xfail(strict=True)` conditioned on a runtime PROBE of the backend (`pd.Series(["ss"]).str.upper() != "SS"`), applied per-case rather than to the whole matrix — everything else there is casing-agnostic and must keep passing on both backends. On pandas 2 the condition is False and the marker is inert, so the cases must PASS; on pandas 3 they must FAIL. Whoever reconciles the backends is told to delete the marker instead of finding a stale skip. Also fixes the id builder for the matrix: it did `t[0]`, which breaks the moment a case becomes a `pytest.param`. Uses `getattr(t, "values", t)[0]` rather than an isinstance against `_pytest.mark.structures.ParameterSet` — that is a private import, and this needs no more than "tuple, or the thing wrapping one". Local: 31 passed with cuDF absent (pandas 2.3.3, marker inert). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015YsqAZQLbqjSDrYSFz2GoB
1 parent bb440f9 commit 458498f

2 files changed

Lines changed: 43 additions & 9 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
2424
- **Seeded typed-hop property projection keeps its fast path through DISTINCT / ORDER BY / SKIP / LIMIT**: those trailing row ops are plain frame operations, so the fast path now delegates them to the canonical chain instead of declining the whole query.
2525

2626
### Changed
27-
- **Seven GFQL test files that ran in NO CI lane at all now run in the polars lane (#1795)**: `bin/test-polars.sh` is the single source of truth for that lane's file list, and a file absent from it was only ever collected by `test-gfql-core` — which installs the `test` extra and therefore has **no polars**, so a module-level `pytest.importorskip("polars")` skipped the whole file there too. The result is the worst kind of green: both lanes pass and the file has executed zero times. Added: `test_engine_polars_call_modality.py` (12 tests), `test_viz_pipeline_conformance.py` (7), `test_engine_polars_gpu.py` (5) — the three named in the audit — plus four more found by the same query rather than by name: `test_engine_polars_narrow_combine.py` (120), `test_engine_polars_semi_key_dedup.py` (36, the #1784 regression suite, which was the ONLY test file for a polars-only change), `index/test_indexed_bindings.py` (87 on the polars lane) and `test_reentry_caller_graph_immutability.py` (18, the #1793 suite — no module-level importorskip, so nothing flagged that its polars parameters ran nowhere). `test_engine_polars_gpu.py` still self-skips without a GPU and is listed deliberately anyway: absent from the list it is not even imported, so a broken import or a renamed helper in it goes unnoticed until someone runs it by hand. **What it surfaced: one CI-red test and no product failures.** `index/test_indexed_bindings.py::test_explicit_polars_gpu_declines_indexed_helper_and_falls_back` had no GPU guard — building the polars frames does not need the RAPIDS stack, so the graph constructed fine and the run died later inside `.gfql()` with `ImportError: GFQL engine='polars-gpu' requires the RAPIDS cudf_polars stack`. Listing the file would therefore have turned the polars lane red; `_native_frames` now `importorskip`s `cudf_polars` for the `polars-gpu` engine so it skips as the environment fact it is. That an unguarded GPU test could sit in the tree unnoticed is the defect this change fixes, in miniature. Running the assembled lane also caught a **pre-existing** trap in a file that was already listed: `index/test_index.py`'s engine list added `polars-gpu` when **`cudf`** imported, but `polars-gpu` is the `cudf_polars` GPU collect target and raises without it — so any box with cuDF and no `cudf_polars` (a real configuration) got **20 fabricated failures** indistinguishable from product breakage. CI never saw it because its polars lane installs neither. The gate now tests `cudf_polars`. 304 passed with cuDF absent (the CI polars lane's configuration) and 394 passed on the dgx GB10 box against `graphistry/test-rapids-official:26.02-gfql-polars` with cuDF 26.02.01 and `cudf_polars` present, i.e. with every cuDF and polars-gpu parameter live. The 37 failures seen on a developer box with `cudf` importable but `libnvrtc.so.12` missing are that environment, not the product — they reproduce nowhere with a working CUDA runtime, which is exactly why they had to be re-run on real hardware before being reported as findings.
27+
- **Seven GFQL test files that ran in NO CI lane at all now run in the polars lane (#1795)**: `bin/test-polars.sh` is the single source of truth for that lane's file list, and a file absent from it was only ever collected by `test-gfql-core` — which installs the `test` extra and therefore has **no polars**, so a module-level `pytest.importorskip("polars")` skipped the whole file there too. The result is the worst kind of green: both lanes pass and the file has executed zero times. Added: `test_engine_polars_call_modality.py` (12 tests), `test_viz_pipeline_conformance.py` (7), `test_engine_polars_gpu.py` (5) — the three named in the audit — plus four more found by the same query rather than by name: `test_engine_polars_narrow_combine.py` (120), `test_engine_polars_semi_key_dedup.py` (36, the #1784 regression suite, which was the ONLY test file for a polars-only change), `index/test_indexed_bindings.py` (87 on the polars lane) and `test_reentry_caller_graph_immutability.py` (18, the #1793 suite — no module-level importorskip, so nothing flagged that its polars parameters ran nowhere). `test_engine_polars_gpu.py` still self-skips without a GPU and is listed deliberately anyway: absent from the list it is not even imported, so a broken import or a renamed helper in it goes unnoticed until someone runs it by hand. **What it surfaced: a real cross-engine divergence, a CI-red test, and a pre-existing fabricated-failure trap.** The divergence first: the polars lane resolves **pandas 3.0.3** while local dev and `test-gfql-core` resolve 2.3.3, and pandas 3's PyArrow-backed `str` dtype does **simple** Unicode case mapping where Python, polars and pandas 2 do **full** mapping — `'ß'.upper()` is `'ẞ'` (U+1E9E) instead of `'SS'`, and `'İ'.lower()` is `'i'` instead of `'i' + U+0307`. So `MATCH (n) WHERE toUpper(n.name) = 'STRASSE'` returns a different row set on pandas 3 than on polars, with no error, and pandas is the oracle the parity suites compare against. Filed as #1812 (it needs an owner call on which mapping is correct) and pinned with a `strict=True` xfail conditioned on a runtime probe of the backend, so the two cases must still PASS on pandas 2 and must FAIL on pandas 3 — whoever reconciles the backends is told to delete the marker rather than finding a stale skip. `index/test_indexed_bindings.py::test_explicit_polars_gpu_declines_indexed_helper_and_falls_back` had no GPU guard — building the polars frames does not need the RAPIDS stack, so the graph constructed fine and the run died later inside `.gfql()` with `ImportError: GFQL engine='polars-gpu' requires the RAPIDS cudf_polars stack`. Listing the file would therefore have turned the polars lane red; `_native_frames` now `importorskip`s `cudf_polars` for the `polars-gpu` engine so it skips as the environment fact it is. That an unguarded GPU test could sit in the tree unnoticed is the defect this change fixes, in miniature. Running the assembled lane also caught a **pre-existing** trap in a file that was already listed: `index/test_index.py`'s engine list added `polars-gpu` when **`cudf`** imported, but `polars-gpu` is the `cudf_polars` GPU collect target and raises without it — so any box with cuDF and no `cudf_polars` (a real configuration) got **20 fabricated failures** indistinguishable from product breakage. CI never saw it because its polars lane installs neither. The gate now tests `cudf_polars`. 304 passed with cuDF absent (the CI polars lane's configuration) and 394 passed on the dgx GB10 box against `graphistry/test-rapids-official:26.02-gfql-polars` with cuDF 26.02.01 and `cudf_polars` present, i.e. with every cuDF and polars-gpu parameter live. The 37 failures seen on a developer box with `cudf` importable but `libnvrtc.so.12` missing are that environment, not the product — they reproduce nowhere with a working CUDA runtime, which is exactly why they had to be re-run on real hardware before being reported as findings.
2828
- **`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.
2929
- **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.
3030

graphistry/tests/compute/gfql/test_viz_pipeline_conformance.py

Lines changed: 42 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,27 @@ def test_panel_state_fuzzer():
395395
# ---- Section 5: case/regex/unicode trick matrix (per-row pin + mirror + parity-or-NIE) ----
396396
# `=~` is ANCHORED (fullmatch); CONTAINS is substring; searchAny default is ci LITERAL
397397
# substring with regex opt-in ((?i)-defaulted). Engine declines ride parity-or-NIE.
398+
#: TRUE when this pandas' string kernels do SIMPLE case mapping instead of Python's FULL
399+
#: case mapping. pandas 3 defaults to a PyArrow-backed ``str`` dtype whose ``utf8_upper`` /
400+
#: ``utf8_lower`` map 'ß' -> 'ẞ' (U+1E9E) and 'İ' -> 'i', where Python (and polars, and
401+
#: pandas 2's object dtype) give 'SS' and 'i' + U+0307. Measured, not assumed:
402+
#: pandas 2.3.3 -> 'SS' / 'i\u0307'; pandas 3.0.3 -> 'ẞ' / 'i'.
403+
#: This is a REAL cross-engine divergence on pandas 3 (see #1812), not a test artifact — the
404+
#: two cases below are the only ones in this matrix whose answer depends on it.
405+
_ARROW_SIMPLE_CASING = pd.Series(["\u00df"]).str.upper().iloc[0] != "SS"
406+
407+
#: Applied per-case rather than to the whole matrix: everything else here is casing-agnostic
408+
#: and must keep passing on both backends. ``strict=True`` on purpose — on pandas 2 the
409+
#: condition is False and the marker is inert, and on pandas 3 the case MUST fail, so whoever
410+
#: reconciles the two backends is told to delete the marker instead of leaving a stale skip.
411+
_CASING_XFAIL = pytest.mark.xfail(
412+
_ARROW_SIMPLE_CASING,
413+
strict=True,
414+
reason="#1812: pandas 3's PyArrow string kernels do SIMPLE case mapping "
415+
"('ss' -> U+1E9E, 'I-dot' -> 'i'), so pandas and polars disagree on toUpper/toLower",
416+
)
417+
418+
398419
def _trick_matrix():
399420
return [
400421
# (?i) vs case-sensitive anchored regex on the case triplet
@@ -433,14 +454,20 @@ def _trick_matrix():
433454
("tolower-eq-no-ss-fold",
434455
"MATCH (n) WHERE toLower(n.name) = 'strasse' RETURN n.id AS id", [9],
435456
lambda nd: nd["name"].str.lower() == "strasse"),
436-
("toupper-eq-ss-fold",
437-
"MATCH (n) WHERE toUpper(n.name) = 'STRASSE' RETURN n.id AS id", [8, 9],
438-
lambda nd: nd["name"].str.upper() == "STRASSE"),
457+
pytest.param(
458+
"toupper-eq-ss-fold",
459+
"MATCH (n) WHERE toUpper(n.name) = 'STRASSE' RETURN n.id AS id", [8, 9],
460+
lambda nd: nd["name"].str.upper() == "STRASSE",
461+
marks=_CASING_XFAIL,
462+
),
439463
# 'İ' (U+0130) lowercases to 'i' + combining dot above (U+0307) — written as an
440464
# explicit escape so editors cannot silently normalize the invisible codepoint
441-
("tolower-turkish-dotted-i",
442-
"MATCH (n) WHERE toLower(n.name) = 'istanbul i\u0307' RETURN n.id AS id", [7],
443-
lambda nd: nd["name"].str.lower() == "istanbul i\u0307"),
465+
pytest.param(
466+
"tolower-turkish-dotted-i",
467+
"MATCH (n) WHERE toLower(n.name) = 'istanbul i\u0307' RETURN n.id AS id", [7],
468+
lambda nd: nd["name"].str.lower() == "istanbul i\u0307",
469+
marks=_CASING_XFAIL,
470+
),
444471
# null cells in the searched/filtered column never match (D3 true nulls)
445472
("searchany-null-cells",
446473
"MATCH (n) WHERE searchAny(n, 'item') RETURN n.id AS id", list(range(12, 30)),
@@ -466,8 +493,15 @@ def _trick_matrix():
466493
]
467494

468495

469-
@pytest.mark.parametrize("label,query,pinned,mirror", _trick_matrix(),
470-
ids=[t[0] for t in _trick_matrix()])
496+
def _trick_ids():
497+
"""Case labels as test ids. Handles the `pytest.param(...)` entries too: a ParameterSet
498+
carries its tuple under `.values`, so a bare `t[0]` would break the moment a case grows a
499+
marker. `getattr` rather than an `isinstance` against `_pytest.mark.structures` — that is
500+
a private import, and this needs no more than "tuple, or the thing wrapping one"."""
501+
return [getattr(t, "values", t)[0] for t in _trick_matrix()]
502+
503+
504+
@pytest.mark.parametrize("label,query,pinned,mirror", _trick_matrix(), ids=_trick_ids())
471505
def test_case_regex_unicode_trick_matrix(label, query, pinned, mirror):
472506
g = _panel_graph()
473507
nd = _panel_nodes()

0 commit comments

Comments
 (0)