Skip to content

Commit e311a86

Browse files
lmeyerovclaude
andcommitted
Merge #1816 into docs/withdraw-q8-win-claim-1825 (linear PR stack, top)
Docs sit at the TOP of the stack so a change-request here cannot block the code PRs underneath it. CHANGELOG hand-resolved: this branch REPLACES #1823's entry rather than adding one, so neither a textual 3-way nor the add-only fallback applies. Built as the parent file with the q8 withdrawal applied in place, asserting the anchor occurs exactly once before the edit and the entry exactly once after. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YYZRXegrALuXd3NHH5evqx
2 parents 46e2548 + d395790 commit e311a86

23 files changed

Lines changed: 3201 additions & 61 deletions

CHANGELOG.md

Lines changed: 5 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)