BUG: quantile segfaults on invalid quantile values

[ghost]

• closes BUG: Groupby.quantile segfaults for quantile > 1 #27470
• waiting for CLN: Centralised _check_percentile #27584 to be resolved (move check_percentiles out of NDFrame)
• waiting for Groupby Array-Type Quantiles Broken in 0.25.0 #27526 to be resolved (Cythonized GroupBy Quantile #20405 cythonized Groupby.quantile with functionality regression)
• whatsnew entry

[jbrockmendel]

fixes this here makes sense. is the underlying problem in numpy or in one of our cython functions?

[WillAyd]

We do a similar validation for NDFrame input via the _check_percentile method - if we were to move that to PandasObject to share across NDFrame and GroupBy would that be even simpler?

[ghost]

is the underlying problem in numpy or in one of our cython functions?

I think this line is the problem
https://github.com/pydata/pandas/blob/d48306e77c0a708e0ee33a2aa1da7e267df52ef6/pandas/_libs/groupby.pyx#L758-L759

it calculates an index by using the quantile value as a fraction of array length.

[ghost]

if we were to move that to PandasObject to share across NDFrame and GroupBy would that be even
simpler?

Yeah, _check_percentile is what this does. I don't think it fits in PandasObject, maybe in lib.pyx instead? it lives on NDFrame but is actually a standalone static method.

[jbrockmendel]

Let's put the check in the cython function, so as to preclude any chance of the segfault showing up, e.g. if we call it from somewhere else

[ghost]

Logical, but thorny. Is the overall approach in pandas to put input validation in cython in the python caller? could also usea python wrapper which is the only cython caller.

@WillAyd suggests reusing NDFrame._check_percentiles instead. It's often inconsistent standards about where validation should happen that creates these sort of bugs, so maybe a bigger issue here.

Whatever there's agreement on is fine by me.

[jbrockmendel]

but thorny

what is thorny about it? I haven't looked at this too specifically, just in principle would rather segfault-prevention happen as close to the fault as possible

[ghost]

blocked by #27526

[ghost]

waiting for #27584

[ghost]

This was a simple fix, but it now depends on too many other small things. I'm sure someone else will get it eventually.