Skip to content

Commit d395790

Browse files
lmeyerovclaude
andcommitted
Merge typing/wave0-polars-frame into perf/gfql-h3-two-hop-count-lazy (linear PR stack)
Stacked so the shared CHANGELOG stops conflicting on every landing: this branch now sits on top of typing/wave0-polars-frame rather than beside it. Files touched by both were 3-way merged with zero conflict markers; CHANGELOG resolved structurally, with this branch's entry re-inserted into the parent's file under the same section. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YYZRXegrALuXd3NHH5evqx
2 parents 315d3cc + 4538b72 commit d395790

22 files changed

Lines changed: 2701 additions & 54 deletions

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Large diffs are not rendered by default.

bin/ci_cypher_surface_guard_baseline.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,5 +13,5 @@
1313
"max_properties": 0
1414
}
1515
},
16-
"lowering_py_max_lines": 9255
16+
"lowering_py_max_lines": 9260
1717
}

bin/test-polars.sh

100755100644
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,9 @@ POLARS_TEST_FILES=(
3434
# only ever run here (the file has no module-level importorskip, so nothing else flags it)
3535
graphistry/tests/compute/gfql/test_exec_context_scoping.py
3636
graphistry/tests/compute/gfql/test_engine_polars_cypher_conformance.py
37+
# aggregate x dtype type contract: the polars params of this file are the ONLY lane where the
38+
# native polars aggregate guard and the raw-polars-exception wrap are exercised
39+
graphistry/tests/compute/gfql/test_aggregate_type_contract.py
3740
graphistry/tests/compute/gfql/test_engine_polars_conformance_matrix.py
3841
graphistry/tests/compute/gfql/test_polars_string_predicate_nonstring.py
3942
graphistry/tests/compute/gfql/cypher/test_order_by_null_placement.py

docs/source/gfql/spec/cypher_mapping.md

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -437,10 +437,25 @@ analysis = g.gfql([
437437
| `LIMIT 10` | `limit(10)` | Row cap |
438438
| `WHERE <row expr>` | `where_rows(expr="...")` | Scalar expression subset |
439439
| `count(*)` | `group_by(keys=[...], aggregations=[("cnt", "count")])` | Grouped count |
440-
| `sum(n.val)` | `group_by(..., aggregations=[("total", "sum", "val")])` | Grouped sum |
440+
| `sum(n.val)` | `group_by(..., aggregations=[("total", "sum", "val")])` | Grouped sum; see [Aggregate input types](#aggregate-input-types) |
441441
| `collect(n.x)` | `group_by(..., aggregations=[("xs", "collect", "x")])` | Nulls excluded from collection |
442442
| Named patterns | `rows(source="alias")` | Scope row table to a named match alias |
443443

444+
### Aggregate input types
445+
446+
`sum` and `avg` accept `INTEGER`, `FLOAT` and `DURATION`, matching Cypher — **plus `BOOLEAN`, which
447+
is a deliberate GFQL extension.** Neo4j rejects it (*"expected Float, Integer or Duration but was
448+
Boolean"*); GFQL accepts it because summing an indicator column is idiomatic in the dataframe
449+
surface GFQL also serves, and every engine already agrees on the answer. `sum` over a boolean
450+
counts the true values; `avg` gives their fraction.
451+
452+
Any other input type **raises**. This is stricter than earlier releases, where a string column
453+
returned its *concatenation* on pandas and leaked a raw polars error on polars — wrong in two
454+
different directions.
455+
456+
Empty and all-null inputs follow Cypher rather than SQL: `sum` returns **0**, `avg` returns
457+
**null**.
458+
444459
## Key Differences
445460

446461
| Feature | Python | Wire Protocol |

graphistry/Engine.py

Lines changed: 51 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,14 @@
1313
# Frame aliases (pandas types for mypy; ``Any`` at runtime). Imported under TYPE_CHECKING
1414
# and referenced via string annotations below so Engine.py — imported very early — never
1515
# triggers ``graphistry.compute`` package init at runtime (would be circular).
16-
from graphistry.compute.typing import DataFrameT, SeriesT
16+
from graphistry.compute.typing import DataFrameT, PolarsFrame, PolarsSeriesT, SeriesT
17+
# TypeGuard, NOT TypeIs (PEP 742). TypeIs additionally narrows the negative branch, and to
18+
# do that soundly it REQUIRES the narrowed type to be consistent with the declared input
19+
# type. ``is_polars_df`` is called on values declared ``DataFrameT``/``SeriesT`` (i.e.
20+
# pandas), and a polars frame is not a subtype of a pandas one, so TypeIs is rejected here
21+
# — it typechecks only where the declared type already contains the narrowed one (as in
22+
# ``polars/dtypes.py:is_lazy``, whose input is already ``PolarsFrame``).
23+
from typing_extensions import TypeGuard
1724

1825

1926
class Engine(Enum):
@@ -196,13 +203,20 @@ def _cudf_from_pandas_best_effort(df: pd.DataFrame, *, validate: Optional[Valida
196203
return out_gdf
197204

198205

199-
def is_polars_df(df: Any) -> bool:
206+
def is_polars_df(df: object) -> "TypeGuard[PolarsFrame]":
200207
"""True if ``df`` is a polars DataFrame or LazyFrame.
201208
202209
Import-light module-name check (polars is an optional dependency, so we avoid importing
203210
it just to ``isinstance``). ``type(df).__module__`` starts with ``polars.`` for both
204211
``pl.DataFrame`` and ``pl.LazyFrame``. Single source of truth — the gfql engine had this
205-
reimplemented in 5 places."""
212+
reimplemented in 5 places.
213+
214+
Declared ``TypeGuard[PolarsFrame]`` rather than ``bool`` so the polars branch of an
215+
engine-dispatching helper actually narrows: callers pass a value declared ``DataFrameT``
216+
(pandas at checking time) and the guard is what proves the polars API is available inside
217+
the branch, with no ``cast`` at the call site. ``object`` (not ``Any``) as the parameter
218+
type is deliberate — it is the widest input a predicate can accept while still checking
219+
its own body."""
206220
return df is not None and "polars" in type(df).__module__
207221

208222

@@ -287,7 +301,10 @@ def df_to_engine(df, engine: Engine, *, validate: Optional[ValidationParam] = No
287301
with target_mode(_tgt):
288302
return _pl_nan_to_null(_lazy_collect(df))
289303
if isinstance(df, pa.Table):
290-
return _pl_nan_to_null(pl.from_arrow(df))
304+
# ``pl.from_arrow`` is declared ``DataFrame | Series`` because polars cannot
305+
# overload it on the pyarrow input type (see polars/convert/general.py); a
306+
# ``pa.Table`` in always yields a DataFrame, so the Series arm is unreachable here.
307+
return _pl_nan_to_null(pl.from_arrow(df)) # type: ignore[arg-type]
291308
pl_validate: ValidationParam = 'strict' if validate is None else validate
292309
if isinstance(df, pd.DataFrame):
293310
return _pl_from_pandas(df, validate=pl_validate, warn=warn)
@@ -300,7 +317,8 @@ def df_to_engine(df, engine: Engine, *, validate: Optional[ValidationParam] = No
300317
if 'cudf' in str(type(df).__module__):
301318
import cudf
302319
if isinstance(df, cudf.DataFrame):
303-
return _pl_nan_to_null(pl.from_arrow(df.to_arrow()))
320+
# see the pa.Table branch above for the from_arrow union
321+
return _pl_nan_to_null(pl.from_arrow(df.to_arrow())) # type: ignore[arg-type]
304322
# dask/spark and anything else: route through pandas
305323
return _pl_from_pandas(df_to_engine(df, Engine.PANDAS), validate=pl_validate, warn=warn)
306324
raise ValueError(f'Only engines pandas/cudf/dask/polars supported, got: {engine}')
@@ -862,39 +880,51 @@ def safe_merge(
862880
# Engine-agnostic series / frame primitives (pandas / cuDF / polars dispatch).
863881
#
864882
# 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()``.
883+
# branches genuinely RETURN polars objects, which cannot be the pandas frame aliases
884+
# the engine-agnostic signature promises, so the localized ``# type: ignore`` lives
885+
# here at the dispatch point and callers get a clean ``SeriesT`` / ``DataFrameT``
886+
# contract with no ``cast()``.
887+
# Since ``is_polars_df`` / ``is_polars_series`` became TypeGuards the polars branch is
888+
# now genuinely CHECKED against the polars API (the ignores below are scoped to the
889+
# return/argument mismatch alone, no longer blanket ``attr-defined``).
868890
# Annotations are strings so the TYPE_CHECKING-only alias import stays runtime-free.
869891
# ---------------------------------------------------------------------------
870892

871893

894+
def is_polars_series(s: object) -> "TypeGuard[PolarsSeriesT]":
895+
"""True if ``s`` is a polars Series. Same import-light module check as ``is_polars_df``
896+
(``polars.series.series``), split out only so the SERIES call sites narrow to
897+
``pl.Series`` instead of to a frame -- the frame guard would wrongly promise
898+
``.height`` / ``.columns`` on a value that is a column."""
899+
return s is not None and "polars" in type(s).__module__
900+
901+
872902
def is_series_like(s: object) -> bool:
873903
"""True for a pandas/cuDF Series (``.dropna``) or a polars Series (module check).
874904
875905
Some engine-agnostic callers accept an ``ids`` Series that is pandas under
876906
``engine='pandas'`` and polars under ``engine='polars'``; both are valid."""
877-
return hasattr(s, "dropna") or is_polars_df(s)
907+
return hasattr(s, "dropna") or is_polars_series(s)
878908

879909

880910
def series_not_null_mask(s: "SeriesT") -> "SeriesT":
881911
"""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]
912+
if is_polars_series(s):
913+
return s.is_not_null() # type: ignore[return-value] # polars Series, not the pandas alias
884914
return s.notna()
885915

886916

887917
def series_filter(s: "SeriesT", mask: "SeriesT") -> "SeriesT":
888918
"""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]
919+
if is_polars_series(s):
920+
return s.filter(mask) # type: ignore[return-value] # polars Series, not the pandas alias
891921
return s[mask].reset_index(drop=True)
892922

893923

894924
def frame_filter(df: "DataFrameT", mask: "SeriesT") -> "DataFrameT":
895925
"""Filter a DataFrame's rows by a boolean mask, engine-aware, dropping the old index."""
896926
if is_polars_df(df):
897-
return df.filter(mask) # type: ignore[attr-defined,no-any-return]
927+
return df.filter(mask) # type: ignore[return-value] # polars frame, not the pandas alias
898928
return df.loc[mask].reset_index(drop=True)
899929

900930

@@ -909,16 +939,18 @@ def ordered_left_join(left: "DataFrameT", right: "DataFrameT", *, on: str) -> "D
909939
if is_polars_df(left):
910940
if not is_polars_df(right):
911941
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]
942+
return left.join(right, on=on, how="left", maintain_order="left") # type: ignore[arg-type,return-value]
913943
return safe_merge(left, right, on=on, how="left")
914944

915945

916946
def row_as_mapping(rows: "DataFrameT", row_index: int) -> Mapping[str, Any]:
917947
"""One frame row as a col->scalar mapping, engine-aware (``row[col]`` works for
918948
both the pandas Series and the polars named-row dict)."""
919949
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]
950+
# ``.row()`` is eager-only; the guard cannot rule out a LazyFrame, and callers of
951+
# this helper are always past collection.
952+
return rows.row(row_index, named=True) # type: ignore[union-attr]
953+
return rows.iloc[row_index] # type: ignore[return-value] # pandas Series is a str->scalar mapping
922954

923955

924956
def assign_constant_columns(df: "DataFrameT", values: Dict[str, Any]) -> "DataFrameT":
@@ -927,14 +959,14 @@ def assign_constant_columns(df: "DataFrameT", values: Dict[str, Any]) -> "DataFr
927959
return df
928960
if is_polars_df(df):
929961
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]
962+
return df.with_columns([pl.lit(v).alias(k) for k, v in values.items()]) # type: ignore[return-value]
931963
return df.assign(**values)
932964

933965

934966
def drop_columns(df: "DataFrameT", cols: Sequence[str]) -> "DataFrameT":
935967
"""Drop columns by name, engine-aware (polars ``drop(list)`` vs pandas ``drop(columns=)``)."""
936968
if is_polars_df(df):
937-
return df.drop(list(cols)) # type: ignore[no-any-return]
969+
return df.drop(list(cols)) # type: ignore[return-value]
938970
return df.drop(columns=list(cols))
939971

940972

0 commit comments

Comments
 (0)