You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
[C++][Python] tdigest and hash_tdigest return NaN for q outside [0, 1]; quantile raises #55372
Describe the bug, including details regarding any error messages, version, and platform.
pyarrow.compute.tdigest and hash_tdigest accept a q outside [0, 1] and return NaN, while quantile rejects the same value with ArrowInvalid. TDigestOptions documents the range as a requirement.
$ python -I td.py # pyarrow 26.0.0 wheel, Python 3.12, Linux x86_64
26.0.0
quantile ArrowInvalid Quantile must be between 0 and 1
tdigest [2.0, nan]
[{'k': 1, 'v_tdigest': [nan]}]
contract: TDigestOptions.q in python/pyarrow/_compute.pyx:2388 ("All values must be in [0, 1]"), and cpp/src/arrow/compute/api_aggregate.h ("probability level of quantile must be between 0 and 1 inclusive").
CheckQuantileOptions (cpp/src/arrow/compute/kernels/aggregate_quantile.cc:482, a99464f on main) returns Status::Invalid("Quantile must be between 0 and 1") for any q outside the range. Neither aggregate_tdigest.cc nor GroupedTDigestImpl in hash_aggregate_numeric.cc (the separate hash_tdigest kernel) checks q, and TDigest::Quantile in cpp/src/arrow/util/tdigest.cc returns NAN for q < 0 || q > 1.
Expected: ArrowInvalid for any q outside [0, 1], from a shared check called by both the tdigest and hash_tdigest inits, with the message quantile uses.
I read the source at a99464f and ran the 26.0.0 wheel, not a source build. I did not check how q=NaN should behave: quantile also lets it through.
I can open a PR with the check and tests in test_compute.py, kernels/aggregate_test.cc and acero/hash_aggregate_test.cc. Would you take it?
Hi @chrikrah, this looks like a clean fix. Are you still planning to open the PR? If you’d rather not, I’d be happy to take it. I’d add the check next to CheckQuantileOptions, call it from both the tdigest and hash_tdigest inits with the same message, and add tests in the three places you listed. I’ll leave q=NaN as it is now unless a maintainer wants it changed.
Activity
Abhi2939 commentedon Nov 4, 2026
Hi @chrikrah, this looks like a clean fix. Are you still planning to open the PR? If you’d rather not, I’d be happy to take it. I’d add the check next to CheckQuantileOptions, call it from both the tdigest and hash_tdigest inits with the same message, and add tests in the three places you listed. I’ll leave q=NaN as it is now unless a maintainer wants it changed.