Skip to content

Commit f8837f4

Browse files
lmeyerovclaude
andcommitted
refactor(gfql): move engine-agnostic frame/series helpers to Engine.py
The reentry executor accumulated a cluster of generic pandas/cuDF/polars dispatch helpers (null mask, series/frame filter, order-preserving left join, constant-column assign, drop-columns, row-as-mapping, series-to-pylist) that have no reentry semantics and belong with safe_merge / df_concat / df_unique in the engine layer. gfql_unified.py had already re-implemented series_to_pylist, proving the leak. Move the eight primitives to graphistry.Engine (public names, no underscore), drop the reentry-local defs, and consolidate the duplicate onto the more-defensive version. SeriesT/DataFrameT typing preserved via a TYPE_CHECKING import + string annotations so Engine.py (imported very early) never triggers graphistry.compute package init at runtime (would be circular). No behavior change: byte-identical dispatch; reentry suite 19/19, cypher/entity subset green, ruff clean, mypy-neutral. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6dQEcjdazEnzuvuwf73ZL
1 parent c257996 commit f8837f4

3 files changed

Lines changed: 138 additions & 117 deletions

File tree

graphistry/Engine.py

Lines changed: 107 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,18 @@
33
import numpy as np
44
import pandas as pd
55
import pyarrow as pa
6-
from typing import Any, List, Optional, Union
6+
from typing import TYPE_CHECKING, Any, Dict, List, Mapping, Optional, Sequence, Union
77
from typing_extensions import Literal
88
from enum import Enum
99

1010
from graphistry.models.types import ValidationParam
1111

12+
if TYPE_CHECKING:
13+
# Frame aliases (pandas types for mypy; ``Any`` at runtime). Imported under TYPE_CHECKING
14+
# and referenced via string annotations below so Engine.py — imported very early — never
15+
# triggers ``graphistry.compute`` package init at runtime (would be circular).
16+
from graphistry.compute.typing import DataFrameT, SeriesT
17+
1218

1319
class Engine(Enum):
1420
PANDAS = 'pandas'
@@ -850,3 +856,103 @@ def safe_merge(
850856
raise ValueError("Must specify either 'on' or both 'left_on' and 'right_on'")
851857

852858
return result
859+
860+
861+
# ---------------------------------------------------------------------------
862+
# Engine-agnostic series / frame primitives (pandas / cuDF / polars dispatch).
863+
#
864+
# Pure per-row/per-column dispatch helpers with no domain knowledge — the polars
865+
# branches genuinely return polars objects mypy can't narrow to the pandas frame
866+
# aliases, so the localized ``# type: ignore`` lives here at the dispatch point and
867+
# callers get a clean ``SeriesT`` / ``DataFrameT`` contract with no ``cast()``.
868+
# Annotations are strings so the TYPE_CHECKING-only alias import stays runtime-free.
869+
# ---------------------------------------------------------------------------
870+
871+
872+
def is_series_like(s: object) -> bool:
873+
"""True for a pandas/cuDF Series (``.dropna``) or a polars Series (module check).
874+
875+
Some engine-agnostic callers accept an ``ids`` Series that is pandas under
876+
``engine='pandas'`` and polars under ``engine='polars'``; both are valid."""
877+
return hasattr(s, "dropna") or is_polars_df(s)
878+
879+
880+
def series_not_null_mask(s: "SeriesT") -> "SeriesT":
881+
"""Non-null boolean mask, engine-aware (polars ``is_not_null`` vs pandas ``notna``)."""
882+
if is_polars_df(s):
883+
return s.is_not_null() # type: ignore[attr-defined,no-any-return]
884+
return s.notna()
885+
886+
887+
def series_filter(s: "SeriesT", mask: "SeriesT") -> "SeriesT":
888+
"""Filter a Series by a boolean mask, engine-aware, dropping the old index (pandas)."""
889+
if is_polars_df(s):
890+
return s.filter(mask) # type: ignore[attr-defined,no-any-return]
891+
return s[mask].reset_index(drop=True)
892+
893+
894+
def frame_filter(df: "DataFrameT", mask: "SeriesT") -> "DataFrameT":
895+
"""Filter a DataFrame's rows by a boolean mask, engine-aware, dropping the old index."""
896+
if is_polars_df(df):
897+
return df.filter(mask) # type: ignore[attr-defined,no-any-return]
898+
return df.loc[mask].reset_index(drop=True)
899+
900+
901+
def ordered_left_join(left: "DataFrameT", right: "DataFrameT", *, on: str) -> "DataFrameT":
902+
"""Left join preserving ``left`` row order, engine-aware. Polars ``.merge`` does not exist;
903+
``safe_merge`` (pandas/cuDF ``.merge``) cannot run on polars frames, so branch to
904+
``.join(..., maintain_order='left')`` which pins the left-row ordering the caller needs.
905+
906+
``right`` may arrive on a different engine than ``left`` (e.g. natively-projected polars
907+
``left`` against a still-pandas base table), so align ``right`` onto ``left``'s engine
908+
before the polars join."""
909+
if is_polars_df(left):
910+
if not is_polars_df(right):
911+
right = df_to_engine(right, Engine.POLARS)
912+
return left.join(right, on=on, how="left", maintain_order="left") # type: ignore[call-arg,no-any-return]
913+
return safe_merge(left, right, on=on, how="left")
914+
915+
916+
def row_as_mapping(rows: "DataFrameT", row_index: int) -> Mapping[str, Any]:
917+
"""One frame row as a col->scalar mapping, engine-aware (``row[col]`` works for
918+
both the pandas Series and the polars named-row dict)."""
919+
if is_polars_df(rows):
920+
return rows.row(row_index, named=True) # type: ignore[attr-defined,no-any-return]
921+
return rows.iloc[row_index]
922+
923+
924+
def assign_constant_columns(df: "DataFrameT", values: Dict[str, Any]) -> "DataFrameT":
925+
"""Broadcast scalar ``values`` as constant columns, engine-aware."""
926+
if not values:
927+
return df
928+
if is_polars_df(df):
929+
import polars as pl
930+
return df.with_columns([pl.lit(v).alias(k) for k, v in values.items()]) # type: ignore[attr-defined,no-any-return]
931+
return df.assign(**values)
932+
933+
934+
def drop_columns(df: "DataFrameT", cols: Sequence[str]) -> "DataFrameT":
935+
"""Drop columns by name, engine-aware (polars ``drop(list)`` vs pandas ``drop(columns=)``)."""
936+
if is_polars_df(df):
937+
return df.drop(list(cols)) # type: ignore[no-any-return]
938+
return df.drop(columns=list(cols))
939+
940+
941+
def series_to_pylist(values: "SeriesT") -> List[Any]:
942+
"""Series -> python list, engine-aware, with defensive arrow/pandas fallbacks."""
943+
if hasattr(values, "to_arrow"):
944+
try:
945+
return list(values.to_arrow().to_pylist())
946+
except Exception:
947+
pass
948+
if hasattr(values, "to_pandas"):
949+
try:
950+
return list(values.to_pandas().tolist())
951+
except Exception:
952+
pass
953+
if hasattr(values, "tolist"):
954+
try:
955+
return list(values.tolist())
956+
except Exception:
957+
pass
958+
return list(values)

graphistry/compute/gfql/cypher/reentry/execution.py

Lines changed: 28 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,22 @@
44

55
from typing import Any, Dict, List, Mapping, Optional, Sequence, Tuple, Union, cast
66

7-
from graphistry.Engine import EngineAbstract, df_concat, df_cons, resolve_engine, safe_merge
7+
from graphistry.Engine import (
8+
EngineAbstract,
9+
assign_constant_columns,
10+
df_concat,
11+
df_cons,
12+
drop_columns,
13+
frame_filter,
14+
is_series_like,
15+
ordered_left_join,
16+
resolve_engine,
17+
row_as_mapping,
18+
safe_merge,
19+
series_filter,
20+
series_not_null_mask,
21+
series_to_pylist,
22+
)
823
from graphistry.Plottable import Plottable
924
from graphistry.compute.exceptions import GFQLValidationError, ErrorCode
1025
from graphistry.compute.gfql.cypher.reentry.naming import _reentry_hidden_column_name
@@ -23,79 +38,6 @@
2338
from graphistry.Engine import is_polars_df as _is_polars_df
2439

2540

26-
def _series_is_series_like(s: object) -> bool:
27-
"""True for a pandas/cuDF Series (``.dropna``) or a polars Series (module check).
28-
29-
The reentry executor recovers carried node identities from a projection-meta ``ids`` Series
30-
that is pandas under ``engine='pandas'`` and polars under ``engine='polars'``; both are valid."""
31-
return hasattr(s, "dropna") or _is_polars_df(s)
32-
33-
34-
# The four helpers below are engine-agnostic (pandas/cuDF/polars); `SeriesT`/`DataFrameT` are the
35-
# repo's frame aliases (pandas types for mypy). The polars branches genuinely return polars objects
36-
# mypy can't narrow to the alias, so the suppression lives here at the dispatch point — callers get
37-
# a clean `SeriesT`/`DataFrameT` contract with no cast().
38-
def _series_not_null_mask(s: SeriesT) -> SeriesT:
39-
"""Non-null boolean mask, engine-aware (polars ``is_not_null`` vs pandas ``notna``)."""
40-
if _is_polars_df(s):
41-
return s.is_not_null() # type: ignore[attr-defined,no-any-return]
42-
return s.notna()
43-
44-
45-
def _series_filter(s: SeriesT, mask: SeriesT) -> SeriesT:
46-
"""Filter a Series by a boolean mask, engine-aware, dropping the old index (pandas)."""
47-
if _is_polars_df(s):
48-
return s.filter(mask) # type: ignore[attr-defined,no-any-return]
49-
return s[mask].reset_index(drop=True)
50-
51-
52-
def _frame_filter(df: DataFrameT, mask: SeriesT) -> DataFrameT:
53-
"""Filter a DataFrame's rows by a boolean mask, engine-aware, dropping the old index."""
54-
if _is_polars_df(df):
55-
return df.filter(mask) # type: ignore[attr-defined,no-any-return]
56-
return df.loc[mask].reset_index(drop=True)
57-
58-
59-
def _ordered_left_join(left: DataFrameT, right: DataFrameT, *, on: str) -> DataFrameT:
60-
"""Left join preserving ``left`` row order, engine-aware. Polars ``.merge`` does not exist;
61-
``safe_merge`` (pandas/cuDF ``.merge``) cannot run on polars frames, so branch to
62-
``.join(..., maintain_order='left')`` which pins the WITH-row ordering the reentry seed needs.
63-
64-
Under ``engine='polars'`` the carried ids come from the natively-projected (polars) prefix
65-
while the base node table can still be pandas (the base graph is converted lazily), so align
66-
``right`` onto ``left``'s engine before the polars join."""
67-
if _is_polars_df(left):
68-
from graphistry.Engine import Engine, df_to_engine
69-
if not _is_polars_df(right):
70-
right = df_to_engine(right, Engine.POLARS)
71-
return left.join(right, on=on, how="left", maintain_order="left") # type: ignore[call-arg,no-any-return]
72-
return safe_merge(left, right, on=on, how="left")
73-
74-
75-
def _reentry_row(prefix_rows: DataFrameT, row_index: int) -> Mapping[str, Any]:
76-
"""One prefix row as a col->scalar mapping, engine-aware (``row[col]`` works for
77-
both the pandas Series and the polars named-row dict)."""
78-
if _is_polars_df(prefix_rows):
79-
return prefix_rows.row(row_index, named=True) # type: ignore[attr-defined,no-any-return]
80-
return prefix_rows.iloc[row_index]
81-
82-
83-
def _assign_constant_columns(df: DataFrameT, values: Dict[str, Any]) -> DataFrameT:
84-
"""Broadcast scalar ``values`` as constant columns, engine-aware."""
85-
if not values:
86-
return df
87-
if _is_polars_df(df):
88-
import polars as pl
89-
return df.with_columns([pl.lit(v).alias(k) for k, v in values.items()]) # type: ignore[attr-defined,no-any-return]
90-
return df.assign(**values)
91-
92-
93-
def _drop_columns(df: Any, cols: Sequence[str]) -> Any:
94-
if _is_polars_df(df):
95-
return df.drop(list(cols))
96-
return df.drop(columns=list(cols))
97-
98-
9941
def _bind_reentry_graph(graph: Plottable, node_rows: Optional[DataFrameT], *, empty_edges: bool = False) -> Plottable:
10042
out = graph.bind()
10143
out._nodes = node_rows
@@ -232,22 +174,14 @@ def _optional_reentry_key(record: Dict[str, Any], columns: Tuple[str, ...]) -> T
232174

233175

234176
def _records_for_columns(df: DataFrameT, columns: Tuple[str, ...]) -> List[Dict[str, Any]]:
235-
values_by_column = {column: _series_to_pylist(cast(SeriesT, df[column])) for column in columns}
177+
values_by_column = {column: series_to_pylist(cast(SeriesT, df[column])) for column in columns}
236178
row_count = len(df)
237179
return [
238180
{column: values_by_column[column][row_index] for column in columns}
239181
for row_index in range(row_count)
240182
]
241183

242184

243-
def _series_to_pylist(values: SeriesT) -> List[Any]:
244-
if hasattr(values, "to_arrow"):
245-
return cast(List[Any], values.to_arrow().to_pylist())
246-
if hasattr(values, "tolist"):
247-
return cast(List[Any], values.tolist())
248-
return list(values)
249-
250-
251185
def _optional_reentry_key_value(value: Any) -> Any:
252186
try:
253187
if value != value:
@@ -323,7 +257,7 @@ def compiled_query_reentry_state(
323257
)
324258
ids = meta["ids"]
325259
id_column = meta["id_column"]
326-
if not _series_is_series_like(ids):
260+
if not is_series_like(ids):
327261
raise reentry_validation_error(
328262
"Cypher MATCH after WITH could not recover carried node identities from the prefix stage",
329263
value=output_name,
@@ -457,10 +391,10 @@ def compiled_query_scalar_reentry_state(
457391
value=missing_column,
458392
suggestion="Project the scalar column explicitly before MATCH re-entry.",
459393
)
460-
row = _reentry_row(prefix_rows, row_index)
394+
row = row_as_mapping(prefix_rows, row_index)
461395
node_rows = cast(
462396
DataFrameT,
463-
_assign_constant_columns(
397+
assign_constant_columns(
464398
base_nodes,
465399
{
466400
_reentry_hidden_column_name(output_name): row[output_name]
@@ -480,7 +414,7 @@ def freeform_broadcast_row_to_nodes(
480414
row_index: int,
481415
) -> Plottable:
482416
"""Broadcast one free-form prefix row's hidden carries onto the base nodes."""
483-
row = _reentry_row(prefix_rows, row_index)
417+
row = row_as_mapping(prefix_rows, row_index)
484418
broadcast_values: Dict[str, Any] = {
485419
_reentry_hidden_column_name(col): row[col]
486420
for col in plan.scalar_columns
@@ -495,11 +429,11 @@ def freeform_broadcast_row_to_nodes(
495429
if broadcast_values:
496430
existing_hidden = [c for c in base_nodes.columns if isinstance(c, str) and c.startswith("__cypher_reentry_")]
497431
node_rows = (
498-
cast(DataFrameT, _drop_columns(base_nodes, existing_hidden))
432+
cast(DataFrameT, drop_columns(base_nodes, existing_hidden))
499433
if existing_hidden
500434
else base_nodes
501435
)
502-
node_rows = cast(DataFrameT, _assign_constant_columns(node_rows, broadcast_values))
436+
node_rows = cast(DataFrameT, assign_constant_columns(node_rows, broadcast_values))
503437
else:
504438
node_rows = cast(DataFrameT, base_nodes)
505439

@@ -548,18 +482,18 @@ def aligned_reentry_rows(
548482
value=output_name,
549483
suggestion="Retry with a direct whole-row carry through WITH or inspect intermediate row-shaping before MATCH re-entry.",
550484
)
551-
if not _series_is_series_like(ids):
485+
if not is_series_like(ids):
552486
raise reentry_validation_error(
553487
"Cypher MATCH after WITH could not align carried node identities from the prefix stage",
554488
value=output_name,
555489
suggestion=REENTRY_WHOLE_ROW_SUGGESTION,
556490
)
557491

558-
non_null_mask = _series_not_null_mask(ids)
559-
carried_ids = _series_filter(ids, non_null_mask)
492+
non_null_mask = series_not_null_mask(ids)
493+
carried_ids = series_filter(ids, non_null_mask)
560494
if prefix_rows is None:
561495
return carried_ids, None
562-
return carried_ids, _frame_filter(prefix_rows, non_null_mask)
496+
return carried_ids, frame_filter(prefix_rows, non_null_mask)
563497

564498

565499
def reentry_carry_payload(
@@ -593,4 +527,4 @@ def ordered_reentry_start_nodes(
593527
id_column: str,
594528
) -> DataFrameT:
595529
# MATCH re-entry must preserve the WITH row order, not the base node-table order.
596-
return _ordered_left_join(carried_node_ids, node_rows, on=id_column)
530+
return ordered_left_join(carried_node_ids, node_rows, on=id_column)

graphistry/compute/gfql_unified.py

Lines changed: 3 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
from types import MappingProxyType
77
from typing import Any, Dict, List, Literal, Mapping, Optional, Sequence, Set, Tuple, Union, cast
88
from graphistry.Plottable import Plottable
9-
from graphistry.Engine import Engine, EngineAbstract, POLARS_ENGINES, df_concat, df_cons, df_to_engine, df_unique, is_polars_df, resolve_engine
9+
from graphistry.Engine import Engine, EngineAbstract, POLARS_ENGINES, df_concat, df_cons, df_to_engine, df_unique, is_polars_df, resolve_engine, series_to_pylist
1010
from graphistry.util import setup_logger
1111
from .ast import ASTObject, ASTLet, ASTNode, ASTEdge, ASTCall
1212
from .chain import Chain, chain as chain_impl
@@ -83,25 +83,6 @@
8383
logger = setup_logger(__name__)
8484

8585

86-
def _series_to_pylist(values: Any) -> List[Any]:
87-
if hasattr(values, "to_arrow"):
88-
try:
89-
return list(values.to_arrow().to_pylist())
90-
except Exception:
91-
pass
92-
if hasattr(values, "to_pandas"):
93-
try:
94-
return list(values.to_pandas().tolist())
95-
except Exception:
96-
pass
97-
if hasattr(values, "tolist"):
98-
try:
99-
return list(values.tolist())
100-
except Exception:
101-
pass
102-
return list(values)
103-
104-
10586
def _is_duplicate_carried_rows_reentry_error(exc: GFQLValidationError) -> bool:
10687
context = getattr(exc, "context", None)
10788
if exc.code != ErrorCode.E108 or not isinstance(context, dict):
@@ -217,8 +198,8 @@ def _apply_optional_null_fill(
217198
language="cypher",
218199
)
219200

220-
base_ids = _series_to_pylist(base_rows_df[node_col])
221-
matched_id_list = _series_to_pylist(matched_ids)
201+
base_ids = series_to_pylist(base_rows_df[node_col])
202+
matched_id_list = series_to_pylist(matched_ids)
222203
if len(base_ids) == actual_rows and base_ids == matched_id_list:
223204
return result
224205

0 commit comments

Comments
 (0)