Change the type of length from c_long to isize (in PySlice::index)

[cmpute]

pyo3/src/types/slice.rs

Line 69 in 3af73fa

pub fn indices(&self, length: c_long) -> PyResult<PySliceIndices> {

The underlying function PySlice_GetIndicesEx has an input length of type Py_ssize_t, which should be mapped to isize, instead of c_long. The conversion from c_long to Py_ssize_t seems unnecessary.

[davidhewitt]

Agreed, a PR to fix this would be greatly appreciated 👍

[davidhewitt]

That said, I assume the length should be non-negative? Maybe usize is better, and we can return an overflow error if the usize is greater than isize::MAX?

[cmpute]

Agree!. And if more changes are allowed, I propose to further change the type of start/stop/length of PySliceIndices all to usize, and impleming using the recommended new APIs PySlice_Unpack and PySlice_AdjustIndices. (according to official C-API docs)

[davidhewitt]

I'm certainly open to making those changes to fit better with what Python offers, but we should consider the compatibility story. There's a lot changing in 0.21 already and I'm cautious of adding too much more.

[cmpute]

I'm certainly open to making those changes to fit better with what Python offers, but we should consider the compatibility story. There's a lot changing in 0.21 already and I'm cautious of adding too much more.

This change is already a break change, but I will try to make the changes small, when I create the PR probably this or next week.