chrikrah opened a new pull request, #55409:
URL: https://github.com/apache/arrow/pull/55409
### Rationale for this change
`tdigest` and `hash_tdigest` accept a `q` outside [0, 1] and return `NaN`,
while `quantile` raises `ArrowInvalid` on the same value. `TDigestOptions` and
`QuantileOptions` document the range as a requirement ("All values must be in
[0, 1]", `python/pyarrow/_compute.pyx`; `api_aggregate.h` says the same).
`q=NaN` also gets past the check in `quantile`, and it aborts all three on a
Debug build. Closes #55372.
### What changes are included in this PR?
`CheckQuantileOptions`'s range loop moves to `ValidateQuantileLevels` in
`aggregate_internal.h`, negated so that `NaN` fails it. `TDigestInit` and
`GroupedTDigestImpl::Init` now call it, with the message `quantile` already
uses.
### Are these changes tested?
Yes. `TestTDigestKernel.InvalidQuantileLevel`,
`TestQuantileKernel.InvalidQuantileLevel` and
`GroupBy.TDigestInvalidQuantileLevel` are new, each with `NaN` and empty input.
`test_quantile` and `test_tdigest` gain cases.
```
# base 6bddeb67e, Debug, g++ 13.3.0, Linux x86_64 (cmake line on request)
$ debug/arrow-compute-aggregate-test
[ PASSED ] 347 tests. # base: 345
$ debug/arrow-acero-hash-aggregate-test
[ PASSED ] 98 tests. # base: 97
# old condition restored (level < 0 || level > 1), each new test run alone
with --gtest_filter: all three abort, exit 134
$ python -m pytest -q ttd.py # test_quantile, test_tdigest copied
out, pyarrow 26.0.0 wheel: both DID NOT RAISE ValueError
# not run: the Python tests against a pyarrow built from this branch, and
the wider C++ suites
```
### Are there any user-facing changes?
Yes. A `q` outside [0, 1], `NaN` included, now raises `ArrowInvalid` from
`tdigest` and `hash_tdigest`. It used to return `NaN`, or `null` on empty or
all-null input.
`quantile` now raises on `q=NaN` too. On the 26.0.0 wheel it returned `NaN`,
or with `interpolation='lower'` or `'nearest'` a real-looking `1.0` on `[1.0,
2.0, 3.0]`.
The check runs at init here and at exec in `quantile`, so a bad option fails
before any data is read. Tell me if you'd rather have it at exec in both.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
