Skip to content

Commit

Permalink
Change the types of PySliceIndices and `PySlice::indices (#3761)
Browse files Browse the repository at this point in the history
* Change the type of `PySliceIndices::slicelength` and `PySlice::indices()`

* Fix example

* Fix fmt
  • Loading branch information
cmpute authored Apr 18, 2024
1 parent 9761abf commit 03c50a1
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 13 deletions.
5 changes: 2 additions & 3 deletions examples/getitem/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
use pyo3::exceptions::PyTypeError;
use pyo3::prelude::*;
use pyo3::types::PySlice;
use std::os::raw::c_long;

#[derive(FromPyObject)]
enum IntOrSlice<'py> {
Expand All @@ -29,7 +28,7 @@ impl ExampleContainer {
} else if let Ok(slice) = key.downcast::<PySlice>() {
// METHOD 1 - the use PySliceIndices to help with bounds checking and for cases when only start or end are provided
// in this case the start/stop/step all filled in to give valid values based on the max_length given
let index = slice.indices(self.max_length as c_long).unwrap();
let index = slice.indices(self.max_length as isize).unwrap();
let _delta = index.stop - index.start;

// METHOD 2 - Do the getattr manually really only needed if you have some special cases for stop/_step not being present
Expand Down Expand Up @@ -62,7 +61,7 @@ impl ExampleContainer {
fn __setitem__(&self, idx: IntOrSlice, value: u32) -> PyResult<()> {
match idx {
IntOrSlice::Slice(slice) => {
let index = slice.indices(self.max_length as c_long).unwrap();
let index = slice.indices(self.max_length as isize).unwrap();
println!(
"Got a slice! {}-{}, step: {}, value: {}",
index.start, index.stop, index.step, value
Expand Down
1 change: 1 addition & 0 deletions newsfragments/3761.changed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Change the type of `PySliceIndices::slicelength` and the `length` parameter of `PySlice::indices()`.
23 changes: 13 additions & 10 deletions src/types/slice.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
use crate::err::{PyErr, PyResult};
use crate::ffi::{self, Py_ssize_t};
use crate::ffi;
use crate::ffi_ptr_ext::FfiPtrExt;
use crate::types::any::PyAnyMethods;
use crate::{Bound, PyAny, PyNativeType, PyObject, Python, ToPyObject};
use std::os::raw::c_long;

/// Represents a Python `slice`.
///
/// Only `c_long` indices supported at the moment by the `PySlice` object.
/// Only `isize` indices supported at the moment by the `PySlice` object.
#[repr(transparent)]
pub struct PySlice(PyAny);

Expand All @@ -22,13 +21,17 @@ pyobject_native_type!(
#[derive(Debug, Eq, PartialEq)]
pub struct PySliceIndices {
/// Start of the slice
///
/// It can be -1 when the step is negative, otherwise it's non-negative.
pub start: isize,
/// End of the slice
///
/// It can be -1 when the step is negative, otherwise it's non-negative.
pub stop: isize,
/// Increment to use when iterating the slice from `start` to `stop`.
pub step: isize,
/// The length of the slice calculated from the original input sequence.
pub slicelength: isize,
pub slicelength: usize,
}

impl PySliceIndices {
Expand Down Expand Up @@ -94,7 +97,7 @@ impl PySlice {
/// assuming a sequence of length `length`, and stores the length of the
/// slice in its `slicelength` member.
#[inline]
pub fn indices(&self, length: c_long) -> PyResult<PySliceIndices> {
pub fn indices(&self, length: isize) -> PyResult<PySliceIndices> {
self.as_borrowed().indices(length)
}
}
Expand All @@ -109,20 +112,19 @@ pub trait PySliceMethods<'py>: crate::sealed::Sealed {
/// Retrieves the start, stop, and step indices from the slice object,
/// assuming a sequence of length `length`, and stores the length of the
/// slice in its `slicelength` member.
fn indices(&self, length: c_long) -> PyResult<PySliceIndices>;
fn indices(&self, length: isize) -> PyResult<PySliceIndices>;
}

impl<'py> PySliceMethods<'py> for Bound<'py, PySlice> {
fn indices(&self, length: c_long) -> PyResult<PySliceIndices> {
// non-negative Py_ssize_t should always fit into Rust usize
fn indices(&self, length: isize) -> PyResult<PySliceIndices> {
unsafe {
let mut slicelength: isize = 0;
let mut start: isize = 0;
let mut stop: isize = 0;
let mut step: isize = 0;
let r = ffi::PySlice_GetIndicesEx(
self.as_ptr(),
length as Py_ssize_t,
length,
&mut start,
&mut stop,
&mut step,
Expand All @@ -133,7 +135,8 @@ impl<'py> PySliceMethods<'py> for Bound<'py, PySlice> {
start,
stop,
step,
slicelength,
// non-negative isize should always fit into usize
slicelength: slicelength as _,
})
} else {
Err(PyErr::fetch(self.py()))
Expand Down

0 comments on commit 03c50a1

Please sign in to comment.