Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change the types of PySliceIndices and `PySlice::indices #3761

Merged
merged 4 commits into from
Apr 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading