Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces specialized aggregators for boolean columns in Ray Data stats, resolving an issue where boolean columns would crash the summary() method due to unsupported PyArrow kernels (like subtraction for standard deviation). It maps boolean data types to _boolean_aggregators instead of _numerical_aggregators and adds comprehensive unit tests to verify this behavior. I have no feedback to provide as there are no review comments to evaluate.
|
This pull request has been automatically marked as stale because it has not had You can always ask for help on our discussion forum or Ray's public slack channel. If you'd like to keep this open, just leave any comment, and the stale label will be removed. |
2b507f5 to
0913560
Compare
|
Not stale — rebased onto current master and re-ran the test suite locally ( |
|
Hi @lonexreb, how hard would it be to support booleans in those preprocessors? I think I would prefer that over skipping (since all booleans can be converted to numbers) |
DataType.bool() was mapped to the numerical aggregators, but Std, ApproximateQuantile, and ZeroPercentage rely on PyArrow kernels (e.g. subtract) that have no boolean implementation, so summary() raised ArrowNotImplementedError on any bool column. Give boolean columns their own aggregator set: count, mean (fraction of true values), min, max, missing-value percentage, and approximate top-k (true/false counts). Also route boolean dtypes to it in the heuristic fallback path. Fixes ray-project#62235 Signed-off-by: lonexreb <reach2shubhankar@gmail.com>
…ing stats
Review feedback: rather than omitting Std/ApproximateQuantile/
ZeroPercentage for boolean columns, treat booleans as 0/1 numbers so
they get the full numerical statistics:
- ArrowBlockColumnAccessor.sum_of_squared_diffs_from_mean casts boolean
columns to float64 (PyArrow has no boolean 'subtract' kernel).
- ZeroPercentage casts boolean columns to int8 before the zero
comparison, in both the per-block path and the vectorized
zero_pct_spec path ('equal(bool, int)' has no kernel).
- DataType.bool() maps back to _numerical_aggregators; the dedicated
boolean aggregator set is removed.
summary() on a bool column now reports count, mean (fraction true),
min, max, std, median, missing_pct, and zero_pct (fraction false).
Signed-off-by: lonexreb <reach2shubhankar@gmail.com>
0913560 to
e77b374
Compare
|
Great suggestion @iamjustinhsu — done in e77b374. Booleans now get the full numerical statistics (treated as 0/1) instead of a reduced set:
Rebased onto current master; |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
Reviewed by Cursor Bugbot for commit e77b374. Configure here.
| arrow_compatible = column_accessor._to_arrow_compatible_container() | ||
| if pa.types.is_boolean(arrow_compatible.type): | ||
| # `equal(bool, int)` has no kernel; treat booleans as 0/1. | ||
| arrow_compatible = pc.cast(arrow_compatible, pa.int8()) |
There was a problem hiding this comment.
Boolean cast assumes Arrow container
Medium Severity
The new boolean handling reads .type from _to_arrow_compatible_container(), but the pandas accessor returns a Python list. ZeroPercentage on a pandas boolean column then fails with AttributeError instead of treating False as zero.
Reviewed by Cursor Bugbot for commit e77b374. Configure here.


Why are these changes needed?
Dataset.summary()crashes withArrowNotImplementedErroron any boolean column:DataType.bool()was mapped to_numerical_aggregators, butStd,ApproximateQuantile, andZeroPercentagerely on PyArrow kernels with no boolean implementation. I probed each default aggregator individually against a bool column on master:Count,Mean,Min,Max,MissingValuePercentage, andApproximateTopKall work;StdandZeroPercentageraiseArrowNotImplementedError(and a boolean quantile is not meaningful).This PR gives boolean columns a dedicated
_boolean_aggregatorsset — count, mean (= fraction of true values), min, max, missing-value percentage, and approximate top-k with k=2 (true/false counts) — and routes boolean dtypes to it in the heuristic fallback path as well.Related issue number
Fixes #62235
Checks
black==22.10.0.test_summary_boolean_column(end-to-end regression:summary()on a bool column with nulls, asserting count/mean/min/max/missing_pct values).test_boolean_aggregatorsunit test; updated the two existing expectations that encoded "boolean is numerical".pytest python/ray/data/tests/test_dataset_stats.py→ 39 passed (run locally against master via Ray nightly wheel +setup-dev.pysymlinks).Notes for reviewers / AI-assistance disclosure