Enhancing CTable#675
Merged
Merged
Conversation
CTable.__arrow_c_stream__ lets pyarrow, DuckDB, Polars, and pandas >= 2.2 consume a CTable directly as a stream of record batches with bounded memory. CTable.from_arrow now also accepts any object implementing the same protocol on ingest (single-argument form), in addition to the existing (schema, batches) form. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Column.__setitem__ and Column.assign() now raise ValueError when called on a view (base is not None), pointing at take()/copy() as the escape hatch. Previously cell writes through a view silently mutated the base table's physical storage while structural mutations already raised; this closes the one unguarded path. CTable.__setitem__ already had the guard. Updated docstrings and the view-mutability tests in test_schema_mutations.py accordingly. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…n (Gap C1)
fillna(value) replaces sentinel/None values with value for scalar,
dictionary, and varlen-scalar columns. dropna(subset=None) returns a
view excluding rows where any nullable column in subset is null,
defaulting to every nullable column; implemented by reusing where().
While building these on top of Column.is_null(), found that
_null_mask_for() never actually detected nulls in timestamp columns:
Column.__getitem__ always decodes the raw int64 sentinel into
np.datetime64('NaT') before it reaches the mask check, which then
compared datetime64 values against the raw int sentinel and silently
matched nothing. Fixed by special-casing datetime64 arrays with
np.isnat(). This also fixes is_null()/null_count() for timestamp
columns (Arrow export already worked by accident, since pyarrow treats
NaT as null natively).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Column operator overloads (__add__ and siblings, __lt__ and siblings) now rewrite their expression when any operand is a nullable int/ timestamp/bool column: - Arithmetic (+ - * / // % **): null propagates, promoting the result to float64/NaN (same promotion pandas' legacy int-null arithmetic uses). Fixes t.x + 1 producing garbage (e.g. INT64_MIN + 1) instead of a null marker. - Comparisons (< <= > >= == !=): SQL WHERE semantics, a null operand never satisfies any comparison. Fixes t[t.x < 0] wrongly including null rows (INT64_MIN < 0 was True) while == against the raw sentinel literal also no longer matches nulls. Both rewrites are sentinel-based (no storage/format changes) and cost nothing for non-nullable columns, which take the original, unwrapped expression path unchanged. Kleene three-valued logic is out of scope (documented). One documented limitation: .sum()/.mean() called directly on a derived expression (e.g. (t.x + 1).sum()) don't skip the promoted NaNs, since that expression is a plain LazyExpr with no memory of which column it came from; Column.sum() on a real column still skips nulls as before. Added a "Nulls in expressions" doc section and tests/ctable/test_null_expressions.py covering the propagation rules, the headline filter-exclusion fix, chained arithmetic, mixed nullable/non-nullable operands, and the zero-overhead guarantee for non-nullable columns. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ap D) D1: group_by(engine=...) now accepts "auto", "numpy" (both alias the current NumPy/Cython chunked implementation), and "jit" (reserved, raises NotImplementedError until a benchmarked JIT path exists) instead of unconditionally rejecting anything but "auto". D2: CTableGroupBy.agg() accepts a custom callable as the op, but only via the named form (output_name=(column, callable[, dtype])) since the auto-named mapping/list forms cannot derive a column name for an arbitrary function. The callable receives a 1-D NumPy array of the group's live, non-null values and is called once per group (plain Python loop -- the "slow but correct" baseline and semantics oracle). Since an arbitrary callable can't be incrementally merged across chunks like the built-in ops, UDF specs bypass the Cython/NumPy fast paths and instead accumulate raw per-group values through the existing generic chunked path, concatenating and calling the UDF once all chunks are read. Output dtype is inferred from every group's result (not just the first), raising a clear error on inconsistent types; an explicit dtype can be given instead. A group with no non-null values never calls the UDF, producing a null result like sum/min/max already do for an all-null group. D3: CTable.apply(func, columns=None, dtype=None, engine="auto") is sugar over blosc2.lazyudf() using the table's raw (full-capacity) column storage as inputs -- the same inputs add_computed_column()/ add_generated_column() already pass to lazyudf() -- with the live-row mask applied once to the result. Window functions, an open third-party engine protocol, and numba integration remain explicitly out of scope per the plan. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Records what landed for each gap this session (Arrow PyCapsule interchange, read-only views, sentinel null fillna/dropna/propagation, UDF aggregations + engine dispatch + apply()), including deviations from the original plan and commit references. Gap E stays parked. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
pandas is an optional dependency in this repo; the module-level
`import pandas as pd` broke test collection in CI environments without
it installed. Moved to pytest.importorskip("pandas") inside the one
test that needs it, and rewrote the other new test that only used
to_pandas() for convenience to compare via the existing col() helper
instead, dropping the pandas dependency entirely there.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Implements the “Enhancing CTable” plan (Gaps A–D) by expanding Arrow interoperability, tightening view mutability semantics, improving sentinel-null behavior (including null propagation through expressions), and adding groupby UDF aggregations plus a convenience CTable.apply() wrapper over lazyudf().
Changes:
- Add Arrow PyCapsule streaming export (
CTable.__arrow_c_stream__) and allowCTable.from_arrow()to ingest any capsule-producer object. - Make all CTable views read-only for value writes (block
Column.__setitem__andColumn.assignon views) and update/extend tests accordingly. - Extend nullable support with
Column.fillna(),CTable.dropna(), timestamp null-detection fix, null-aware arithmetic/comparisons, and groupby named UDF aggregations +CTable.apply().
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/ctable/test_schema_mutations.py | Updates/adds view-mutation tests to enforce read-only view semantics. |
| tests/ctable/test_nullable.py | Adds fillna/dropna tests and a regression test for timestamp sentinel null detection. |
| tests/ctable/test_null_expressions.py | New tests for sentinel-based null propagation in Column expressions and documented reduction limitation. |
| tests/ctable/test_groupby.py | Adds engine validation tests and named-form UDF aggregation tests (incl. pandas reference checks). |
| tests/ctable/test_ctable_apply.py | New tests for CTable.apply() behavior vs direct lazyudf() usage. |
| tests/ctable/test_arrow_interop.py | Adds PyCapsule export/import tests and optional DuckDB/Polars interop checks. |
| src/blosc2/groupby.py | Adds named UDF aggregation support, dtype inference, and execution plumbing in the chunked groupby path. |
| src/blosc2/ctable.py | Adds view write guards, fillna/dropna, timestamp null mask fix, null-aware arithmetic/comparisons, Arrow stream export, capsule-producer ingest, and CTable.apply(). |
| plans/enhancing-ctable.md | Adds the full implementation plan + recorded implementation notes for Gaps A–D. |
| doc/reference/ctable.rst | Documents null semantics in expressions, dropna/apply/UDF aggregation docs, and Arrow PyCapsule protocol support. |
| doc/guides/optimization_tips.md | Minor formatting/line-wrapping adjustment in docs. |
| CMakeLists.txt | Bumps bundled C-Blosc2 version to v3.2.3. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1801
to
+1804
| group_values = np.concatenate(chunks) | ||
| try: | ||
| result = _python_scalar(np.asarray(spec.udf(group_values))) | ||
| except Exception as exc: |
Comment on lines
+1838
to
+1840
| try: | ||
| arr = np.asarray(results) | ||
| except ValueError as exc: |
- _final_rows() was wrapping the UDF's return value in np.asarray() before passing it to _python_scalar(). Since _python_scalar() only unwraps np.generic (NumPy scalar types), a 0-D ndarray produced by that wrapping was left as-is instead of becoming a plain Python/ NumPy scalar. Pass the UDF result through directly instead. - _infer_udf_spec() silently inferred float64 from an empty results list (e.g. an empty table, or every group all-null so the UDF was never called), even though there's nothing to infer from. Now raises a clear error pointing at the explicit-dtype escape hatch, matching the existing inconsistent-types error. Added regression tests for both. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Comment on lines
+1809
to
+1810
| row[spec.output_col] = result | ||
| udf_results[spec.output_col].append(result) |
Comment on lines
+1823
to
+1826
| null_value = _null_output_value(spec.explicit_dtype) | ||
| for row in rows: | ||
| if row[spec.output_col] is _empty_udf_group: | ||
| row[spec.output_col] = null_value |
Comment on lines
+1779
to
+1782
| # Sentinel distinguishing "group had zero non-null values, UDF was | ||
| # never called" from a UDF legitimately returning None; patched to a | ||
| # real null value once the output dtype is known, below. | ||
| _empty_udf_group = object() |
Copilot review follow-up: a UDF returning None for some (but not all) groups was appended to udf_results as-is, which made np.asarray(results) produce an object array and _infer_udf_spec() raise "inconsistent or unsupported types" -- misleading for a UDF that legitimately signals "no meaningful value" by returning None, and the _empty_udf_group docstring already (incorrectly) claimed this case was distinguished and handled. None now gets the same treatment as an empty group: patched to the output dtype's null value and excluded from dtype inference, instead of poisoning it. Updated the _empty_udf_group comment to describe what's actually implemented. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…m_arrow() argument-form errors - CTable.apply() annotation/docstring said blosc2.NDArray; it returns np.ndarray (boolean-mask indexing materializes). Document it, and validate column names up front so unknown/computed columns raise a clear ValueError instead of a raw KeyError. - from_arrow() now raises clear TypeErrors for the two wrong argument forms (capsule producer plus batches; plain schema without batches). - Docs: warn that ~(t.price > 0) selects null rows (nulls fold to False before inversion) and show the notnull() escape hatch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Groupby UDF aggregations: chunk-boundary-straddling groups (chunk_size=2 exercises the per-group raw-value accumulation and the concatenate-then-call-once path), UDF called exactly once per group, UDF agg on a filtered view, empty table with explicit dtype, groupby object reuse. - Null expressions: != on a nullable float (the one comparison where IEEE NaN and SQL semantics disagree), </> on nullable floats, timestamp comparisons excluding the NaT sentinel, reverse operators (100 - t.x), floordiv/mod/pow propagation, nan**0 == 1.0 not resurrecting nulls, and a pin of ~(cmp) selecting null rows. - Arrow capsule: empty-table export, ingesting another CTable, and ingesting a filtered view via from_arrow(capsule). - fillna on a view (view rows only, base untouched) and on a non-nullable column (identity). - Docs: the ~(cmp) escape hatch is the complementary comparison; '& notnull()' does not compose (physical- vs live-length mismatch), verified empirically. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comparing or operating a column/array against np.nan (or np.inf) crashed with "NameError: name 'nan' is not defined": the plain-float scalar was embedded into the expression string via repr(), producing a bare 'nan' name that neither numexpr nor the python-eval fallback namespace defines (numexpr has no non-finite literals at all). Fix, in layers: - LazyExpr.__init__: retype non-finite Python floats to np.float64 so they take the existing typed-scalar branch and ride as named operands (numexpr fast path handles scalar operands fine); same named-operand treatment in the funcs_2args branch, where stringifying also caused infinite recursion via the blosc2-functions eval fallback (maximum(a, np.nan) rebuilt the same broken expression forever). - Eval-namespace backstop for expression strings that spell nan/inf as bare names (user string expressions, update_expr's scalar reprs): define them in safe_blosc2_globals, safe_numpy_globals, and get_expr_globals, skip them in the operand-rebasing OperandVisitor (unless an operand really has that name), and treat them as scalars in the ShapeInferencer. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…D1 follow-up) The D1 benchmark gate (1e7 rows, low-cardinality keys) showed the JIT engine idea has nothing to win: int keys already beat pandas (50 vs 61 ms) on the Cython fast paths. The real gap was string keys: 1157 ms vs pandas' 149 ms, with 0.94 s spent in np.unique argsorting U8 keys (32 bytes of UTF-32 per comparison) inside _factorize_keys. _factorize_fixed_width_str hashes each row's raw bytes into one uint64, factorizes the integers instead, and recovers the group strings from one representative row each. A vectorized verify pass keeps it exact: on a hash collision it falls back to plain np.unique, so the output contract (uniques sorted ascending + inverse) is bit-identical and no caller can tell the difference. String-key sum: 1157 -> 737 ms; every caller benefits through the default engine, no engine= switch involved. engine="jit" stays NotImplementedError: miniexpr is elementwise (no grouped-scatter primitive), and the plan's own merge gate rejects a JIT path that cannot beat engine="numpy". Bench script recorded as bench/ctable/bench_groupby_keys.py; plan notes updated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…b deviation) (t.score + 1).sum() previously NaN-poisoned: the C2b rewrite returned a plain LazyExpr with no memory of nullability. Arithmetic involving a nullable column now returns NullableExpr, a thin wrapper carrying the owning table and the null predicate, whose sum/mean/min/max/std skip nulls and dead physical rows exactly like the corresponding Column reductions (same lazy masked-reduction engine, same all-null conventions: sum 0.0, mean/std NaN, min/max ValueError). The wrapper carries the predicate itself rather than deriving nullness as isnan(expr): blosc2.isnan() on a where()-carrying LazyExpr silently returns wrong results (pre-existing lazyexpr quirk), and the predicate keeps nulls distinct from NaNs the arithmetic itself produces (0/0). Chaining keeps the wrapper and ORs predicates; Column operands re-enter via Column.__r<op>__ so their own sentinel rewrite applies; ** re-patches nulls (nan**0 == 1.0 would resurrect them); != guards both operands' predicates. Everything else delegates to the wrapped expression, so filters, compute(), and operand use are unchanged. The limitation-pinning test is replaced by positive tests (pandas parity, chained/mixed operands, deleted rows, views, all-null); docs and plan notes updated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Docstrings must be self-documenting: a reader of the code should not need plans/enhancing-ctable.md to understand what a class or method does. The plan file keeps its own cross-references; the code now states the semantics directly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment on lines
+9727
to
+9733
| names = columns if columns is not None else list(self.col_names) | ||
| missing = [n for n in names if self._logical_to_physical_name(n) not in self._cols] | ||
| if missing: | ||
| raise ValueError( | ||
| f"apply() only accepts stored columns, got {missing!r}. " | ||
| f"Stored columns: {list(self.col_names)!r}." | ||
| ) |
Comment on lines
520
to
524
| def visit_Name(self, node): | ||
| if node.id in ("nan", "inf"): # non-finite float literals: scalars | ||
| return () | ||
| if node.id not in self.shapes: | ||
| raise ValueError(f"Unknown symbol: {node.id}") |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes Gaps A–D from plans/enhancing-ctable.md (pandas-3-inspired feature gaps for CTable). Gap E (col()) stays parked per the plan.
Gap A — Arrow PyCapsule interchange. CTable.arrow_c_stream lets pyarrow, DuckDB, Polars, and pandas ≥2.2 consume a CTable directly as a streaming, bounded-memory batch source. from_arrow() now also accepts any single capsule-producer object (pyarrow Table/RecordBatchReader, Polars DataFrame, DuckDB result, another CTable) in addition to the existing (schema, batches) form.
Gap B — Read-only views. Column.setitem/assign() now raise ValueError when called on a view, pointing at take()/copy(). Previously cell writes silently mutated the base table's physical storage while structural mutations already raised — this closes that one gap.
Gap C — Sentinel-null story. Added Column.fillna() and CTable.dropna(). Fixed a real pre-existing bug where is_null()/null_count() never worked for timestamp columns (sentinel decodes to NaT before the mask check compares it). Implemented sentinel-based null propagation through Column arithmetic/comparisons: arithmetic promotes nullable int/timestamp results to NaN, comparisons exclude nulls (SQL semantics) — fixing t[t.x < 0] wrongly including null rows. One documented limitation: .sum() on a derived expression like (t.x + 1) doesn't auto-skip nulls (no new Column-like wrapper was built for it — flagged as a possible follow-up).
Gap D — UDF aggregations & engine dispatch. group_by(engine=...) accepts "auto"/"numpy" (current path) and "jit" (reserved, raises NotImplementedError — no JIT execution engine was built/benchmarked this round). agg()'s named form now accepts a custom callable (output_name=(col, fn[, dtype])), executed once per group with nulls pre-filtered, dtype inferred/validated across all groups. Added CTable.apply() as thin sugar over blosc2.lazyudf().
All changes are additive/backward-compatible except Gap B (intentional behavior fix) and the timestamp null-detection fix in Gap C (bug fix). Full test suite (7359 tests) passes.