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

migrate many FromPyObject implementations to Bound API #3784

Merged
merged 2 commits into from
Feb 4, 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
10 changes: 5 additions & 5 deletions pyo3-benches/benches/bench_dict.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use codspeed_criterion_compat::{criterion_group, criterion_main, Bencher, Criter
use pyo3::types::IntoPyDict;
use pyo3::{prelude::*, types::PyMapping};
use std::collections::{BTreeMap, HashMap};
use std::hint::black_box;

fn iter_dict(b: &mut Bencher<'_>) {
Python::with_gil(|py| {
Expand Down Expand Up @@ -71,13 +72,12 @@ fn extract_hashbrown_map(b: &mut Bencher<'_>) {
fn mapping_from_dict(b: &mut Bencher<'_>) {
Python::with_gil(|py| {
const LEN: usize = 100_000;
let dict = (0..LEN as u64)
let dict = &(0..LEN as u64)
.map(|i| (i, i * 2))
.into_py_dict(py)
.to_object(py);
b.iter(|| {
let _: &PyMapping = dict.extract(py).unwrap();
});
.to_object(py)
.into_bound(py);
b.iter(|| black_box(dict).downcast::<PyMapping>().unwrap());
});
}

Expand Down
9 changes: 4 additions & 5 deletions pyo3-benches/benches/bench_list.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::hint::black_box;

use codspeed_criterion_compat::{criterion_group, criterion_main, Bencher, Criterion};

use pyo3::prelude::*;
Expand Down Expand Up @@ -56,11 +58,8 @@ fn list_get_item_unchecked(b: &mut Bencher<'_>) {
fn sequence_from_list(b: &mut Bencher<'_>) {
Python::with_gil(|py| {
const LEN: usize = 50_000;
let list = PyList::new_bound(py, 0..LEN).to_object(py);
b.iter(|| {
let seq: &PySequence = list.extract(py).unwrap();
seq
});
let list = &PyList::new_bound(py, 0..LEN);
b.iter(|| black_box(list).downcast::<PySequence>().unwrap());
});
}

Expand Down
2 changes: 1 addition & 1 deletion pyo3-benches/benches/bench_tuple.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ fn sequence_from_tuple(b: &mut Bencher<'_>) {
Python::with_gil(|py| {
const LEN: usize = 50_000;
let tuple = PyTuple::new_bound(py, 0..LEN).to_object(py);
b.iter(|| tuple.extract::<&PySequence>(py).unwrap());
b.iter(|| tuple.downcast::<PySequence>(py).unwrap());
});
}

Expand Down
4 changes: 2 additions & 2 deletions pyo3-macros-backend/src/frompyobject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -568,8 +568,8 @@ pub fn build_derive_from_pyobject(tokens: &DeriveInput) -> Result<TokenStream> {
let lt_param = if let Some(lt) = verify_and_get_lifetime(generics)? {
lt.clone()
} else {
trait_generics.params.push(parse_quote!('source));
parse_quote!('source)
trait_generics.params.push(parse_quote!('py));
parse_quote!('py)
};
let mut where_clause: syn::WhereClause = parse_quote!(where);
for param in generics.type_params() {
Expand Down
7 changes: 4 additions & 3 deletions src/buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
// DEALINGS IN THE SOFTWARE.

//! `PyBuffer` implementation
use crate::instance::Bound;
use crate::{err, exceptions::PyBufferError, ffi, FromPyObject, PyAny, PyResult, Python};
use std::marker::PhantomData;
use std::os::raw;
Expand Down Expand Up @@ -181,9 +182,9 @@ pub unsafe trait Element: Copy {
fn is_compatible_format(format: &CStr) -> bool;
}

impl<'source, T: Element> FromPyObject<'source> for PyBuffer<T> {
fn extract(obj: &PyAny) -> PyResult<PyBuffer<T>> {
Self::get(obj)
impl<'py, T: Element> FromPyObject<'py> for PyBuffer<T> {
fn extract_bound(obj: &Bound<'_, PyAny>) -> PyResult<PyBuffer<T>> {
Self::get(obj.as_gil_ref())
}
}

Expand Down
34 changes: 17 additions & 17 deletions src/conversion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ pub trait IntoPy<T>: Sized {
/// depend on the lifetime of the `obj` or the `prepared` variable.
///
/// For example, when extracting `&str` from a Python byte string, the resulting string slice will
/// point to the existing string data (lifetime: `'source`).
/// point to the existing string data (lifetime: `'py`).
/// On the other hand, when extracting `&str` from a Python Unicode string, the preparation step
/// will convert the string to UTF-8, and the resulting string slice will have lifetime `'prepared`.
/// Since which case applies depends on the runtime type of the Python object,
Expand All @@ -219,20 +219,20 @@ pub trait IntoPy<T>: Sized {
/// has two methods `extract` and `extract_bound` which are defaulted to call each other. To avoid
/// infinite recursion, implementors must implement at least one of these methods. The recommendation
/// is to implement `extract_bound` and leave `extract` as the default implementation.
pub trait FromPyObject<'source>: Sized {
pub trait FromPyObject<'py>: Sized {
/// Extracts `Self` from the source GIL Ref `obj`.
///
/// Implementors are encouraged to implement `extract_bound` and leave this method as the
/// default implementation, which will forward calls to `extract_bound`.
fn extract(ob: &'source PyAny) -> PyResult<Self> {
fn extract(ob: &'py PyAny) -> PyResult<Self> {
Self::extract_bound(&ob.as_borrowed())
}

/// Extracts `Self` from the bound smart pointer `obj`.
///
/// Implementors are encouraged to implement this method and leave `extract` defaulted, as
/// this will be most compatible with PyO3's future API.
fn extract_bound(ob: &Bound<'source, PyAny>) -> PyResult<Self> {
fn extract_bound(ob: &Bound<'py, PyAny>) -> PyResult<Self> {
Self::extract(ob.clone().into_gil_ref())
}

Expand Down Expand Up @@ -308,56 +308,56 @@ impl<T: Copy + IntoPy<PyObject>> IntoPy<PyObject> for Cell<T> {
}
}

impl<'a, T: FromPyObject<'a>> FromPyObject<'a> for Cell<T> {
fn extract(ob: &'a PyAny) -> PyResult<Self> {
impl<'py, T: FromPyObject<'py>> FromPyObject<'py> for Cell<T> {
fn extract(ob: &'py PyAny) -> PyResult<Self> {
T::extract(ob).map(Cell::new)
}
}

impl<'a, T> FromPyObject<'a> for &'a PyCell<T>
impl<'py, T> FromPyObject<'py> for &'py PyCell<T>
where
T: PyClass,
{
fn extract(obj: &'a PyAny) -> PyResult<Self> {
fn extract(obj: &'py PyAny) -> PyResult<Self> {
obj.downcast().map_err(Into::into)
}
}

impl<'a, T> FromPyObject<'a> for T
impl<T> FromPyObject<'_> for T
where
T: PyClass + Clone,
{
fn extract(obj: &'a PyAny) -> PyResult<Self> {
fn extract(obj: &PyAny) -> PyResult<Self> {
let cell: &PyCell<Self> = obj.downcast()?;
Ok(unsafe { cell.try_borrow_unguarded()?.clone() })
}
}

impl<'a, T> FromPyObject<'a> for PyRef<'a, T>
impl<'py, T> FromPyObject<'py> for PyRef<'py, T>
where
T: PyClass,
{
fn extract(obj: &'a PyAny) -> PyResult<Self> {
fn extract(obj: &'py PyAny) -> PyResult<Self> {
let cell: &PyCell<T> = obj.downcast()?;
cell.try_borrow().map_err(Into::into)
}
}

impl<'a, T> FromPyObject<'a> for PyRefMut<'a, T>
impl<'py, T> FromPyObject<'py> for PyRefMut<'py, T>
where
T: PyClass<Frozen = False>,
{
fn extract(obj: &'a PyAny) -> PyResult<Self> {
fn extract(obj: &'py PyAny) -> PyResult<Self> {
let cell: &PyCell<T> = obj.downcast()?;
cell.try_borrow_mut().map_err(Into::into)
}
}

impl<'a, T> FromPyObject<'a> for Option<T>
impl<'py, T> FromPyObject<'py> for Option<T>
where
T: FromPyObject<'a>,
T: FromPyObject<'py>,
{
fn extract(obj: &'a PyAny) -> PyResult<Self> {
fn extract(obj: &'py PyAny) -> PyResult<Self> {
if obj.as_ptr() == unsafe { ffi::Py_None() } {
Ok(None)
} else {
Expand Down
41 changes: 20 additions & 21 deletions src/conversions/chrono.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@
use crate::exceptions::{PyTypeError, PyUserWarning, PyValueError};
#[cfg(Py_LIMITED_API)]
use crate::sync::GILOnceCell;
#[cfg(not(Py_LIMITED_API))]
use crate::types::any::PyAnyMethods;
#[cfg(not(Py_LIMITED_API))]
use crate::types::datetime::timezone_from_offset;
Expand All @@ -52,9 +51,9 @@ use crate::types::{
PyTzInfo, PyTzInfoAccess,
};
#[cfg(Py_LIMITED_API)]
use crate::{intern, PyDowncastError};
use crate::{intern, DowncastError};
use crate::{
FromPyObject, IntoPy, PyAny, PyErr, PyNativeType, PyObject, PyResult, Python, ToPyObject,
Bound, FromPyObject, IntoPy, PyAny, PyErr, PyNativeType, PyObject, PyResult, Python, ToPyObject,
};
use chrono::offset::{FixedOffset, Utc};
use chrono::{
Expand Down Expand Up @@ -109,14 +108,14 @@ impl IntoPy<PyObject> for Duration {
}

impl FromPyObject<'_> for Duration {
fn extract(ob: &PyAny) -> PyResult<Duration> {
fn extract_bound(ob: &Bound<'_, PyAny>) -> PyResult<Duration> {
// Python size are much lower than rust size so we do not need bound checks.
// 0 <= microseconds < 1000000
// 0 <= seconds < 3600*24
// -999999999 <= days <= 999999999
#[cfg(not(Py_LIMITED_API))]
let (days, seconds, microseconds) = {
let delta: &PyDelta = ob.downcast()?;
let delta = ob.downcast::<PyDelta>()?;
(
delta.get_days().into(),
delta.get_seconds().into(),
Expand Down Expand Up @@ -166,10 +165,10 @@ impl IntoPy<PyObject> for NaiveDate {
}

impl FromPyObject<'_> for NaiveDate {
fn extract(ob: &PyAny) -> PyResult<NaiveDate> {
fn extract_bound(ob: &Bound<'_, PyAny>) -> PyResult<NaiveDate> {
#[cfg(not(Py_LIMITED_API))]
{
let date: &PyDate = ob.downcast()?;
let date = ob.downcast::<PyDate>()?;
py_date_to_naive_date(date)
}
#[cfg(Py_LIMITED_API)]
Expand Down Expand Up @@ -211,10 +210,10 @@ impl IntoPy<PyObject> for NaiveTime {
}

impl FromPyObject<'_> for NaiveTime {
fn extract(ob: &PyAny) -> PyResult<NaiveTime> {
fn extract_bound(ob: &Bound<'_, PyAny>) -> PyResult<NaiveTime> {
#[cfg(not(Py_LIMITED_API))]
{
let time: &PyTime = ob.downcast()?;
let time = ob.downcast::<PyTime>()?;
py_time_to_naive_time(time)
}
#[cfg(Py_LIMITED_API)]
Expand All @@ -238,9 +237,9 @@ impl IntoPy<PyObject> for NaiveDateTime {
}

impl FromPyObject<'_> for NaiveDateTime {
fn extract(dt: &PyAny) -> PyResult<NaiveDateTime> {
fn extract_bound(dt: &Bound<'_, PyAny>) -> PyResult<NaiveDateTime> {
#[cfg(not(Py_LIMITED_API))]
let dt: &PyDateTime = dt.downcast()?;
let dt = dt.downcast::<PyDateTime>()?;
#[cfg(Py_LIMITED_API)]
check_type(dt, &DatetimeTypes::get(dt.py()).datetime, "PyDateTime")?;

Expand Down Expand Up @@ -276,10 +275,10 @@ impl<Tz: TimeZone> IntoPy<PyObject> for DateTime<Tz> {
}
}

impl<Tz: TimeZone + for<'a> FromPyObject<'a>> FromPyObject<'_> for DateTime<Tz> {
fn extract(dt: &PyAny) -> PyResult<DateTime<Tz>> {
impl<Tz: TimeZone + for<'py> FromPyObject<'py>> FromPyObject<'_> for DateTime<Tz> {
fn extract_bound(dt: &Bound<'_, PyAny>) -> PyResult<DateTime<Tz>> {
#[cfg(not(Py_LIMITED_API))]
let dt: &PyDateTime = dt.downcast()?;
let dt = dt.downcast::<PyDateTime>()?;
#[cfg(Py_LIMITED_API)]
check_type(dt, &DatetimeTypes::get(dt.py()).datetime, "PyDateTime")?;

Expand Down Expand Up @@ -339,7 +338,7 @@ impl FromPyObject<'_> for FixedOffset {
///
/// Note that the conversion will result in precision lost in microseconds as chrono offset
/// does not supports microseconds.
fn extract(ob: &PyAny) -> PyResult<FixedOffset> {
fn extract_bound(ob: &Bound<'_, PyAny>) -> PyResult<FixedOffset> {
#[cfg(not(Py_LIMITED_API))]
let ob: &PyTzInfo = ob.extract()?;
#[cfg(Py_LIMITED_API)]
Expand Down Expand Up @@ -378,7 +377,7 @@ impl IntoPy<PyObject> for Utc {
}

impl FromPyObject<'_> for Utc {
fn extract(ob: &PyAny) -> PyResult<Utc> {
fn extract_bound(ob: &Bound<'_, PyAny>) -> PyResult<Utc> {
let py_utc = timezone_utc(ob.py());
if ob.eq(py_utc)? {
Ok(Utc)
Expand Down Expand Up @@ -480,7 +479,7 @@ fn py_date_to_naive_date(py_date: &impl PyDateAccess) -> PyResult<NaiveDate> {
}

#[cfg(Py_LIMITED_API)]
fn py_date_to_naive_date(py_date: &PyAny) -> PyResult<NaiveDate> {
fn py_date_to_naive_date(py_date: &Bound<'_, PyAny>) -> PyResult<NaiveDate> {
NaiveDate::from_ymd_opt(
py_date.getattr(intern!(py_date.py(), "year"))?.extract()?,
py_date.getattr(intern!(py_date.py(), "month"))?.extract()?,
Expand All @@ -501,7 +500,7 @@ fn py_time_to_naive_time(py_time: &impl PyTimeAccess) -> PyResult<NaiveTime> {
}

#[cfg(Py_LIMITED_API)]
fn py_time_to_naive_time(py_time: &PyAny) -> PyResult<NaiveTime> {
fn py_time_to_naive_time(py_time: &Bound<'_, PyAny>) -> PyResult<NaiveTime> {
NaiveTime::from_hms_micro_opt(
py_time.getattr(intern!(py_time.py(), "hour"))?.extract()?,
py_time
Expand All @@ -518,9 +517,9 @@ fn py_time_to_naive_time(py_time: &PyAny) -> PyResult<NaiveTime> {
}

#[cfg(Py_LIMITED_API)]
fn check_type(value: &PyAny, t: &PyObject, type_name: &'static str) -> PyResult<()> {
if !value.is_instance(t.as_ref(value.py()))? {
return Err(PyDowncastError::new(value, type_name).into());
fn check_type(value: &Bound<'_, PyAny>, t: &PyObject, type_name: &'static str) -> PyResult<()> {
if !value.is_instance(t.bind(value.py()))? {
return Err(DowncastError::new(value, type_name).into());
}
Ok(())
}
Expand Down
7 changes: 5 additions & 2 deletions src/conversions/chrono_tz.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,11 @@
//! ```
use crate::exceptions::PyValueError;
use crate::sync::GILOnceCell;
use crate::types::any::PyAnyMethods;
use crate::types::PyType;
use crate::{intern, FromPyObject, IntoPy, Py, PyAny, PyObject, PyResult, Python, ToPyObject};
use crate::{
intern, Bound, FromPyObject, IntoPy, Py, PyAny, PyObject, PyResult, Python, ToPyObject,
};
use chrono_tz::Tz;
use std::str::FromStr;

Expand All @@ -59,7 +62,7 @@ impl IntoPy<PyObject> for Tz {
}

impl FromPyObject<'_> for Tz {
fn extract(ob: &PyAny) -> PyResult<Tz> {
fn extract_bound(ob: &Bound<'_, PyAny>) -> PyResult<Tz> {
Tz::from_str(ob.getattr(intern!(ob.py(), "key"))?.extract()?)
.map_err(|e| PyValueError::new_err(e.to_string()))
}
Expand Down
11 changes: 6 additions & 5 deletions src/conversions/either.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@
#[cfg(feature = "experimental-inspect")]
use crate::inspect::types::TypeInfo;
use crate::{
exceptions::PyTypeError, FromPyObject, IntoPy, PyAny, PyObject, PyResult, Python, ToPyObject,
exceptions::PyTypeError, types::any::PyAnyMethods, Bound, FromPyObject, IntoPy, PyAny,
PyObject, PyResult, Python, ToPyObject,
};
use either::Either;

Expand Down Expand Up @@ -81,13 +82,13 @@ where
}

#[cfg_attr(docsrs, doc(cfg(feature = "either")))]
impl<'source, L, R> FromPyObject<'source> for Either<L, R>
impl<'py, L, R> FromPyObject<'py> for Either<L, R>
where
L: FromPyObject<'source>,
R: FromPyObject<'source>,
L: FromPyObject<'py>,
R: FromPyObject<'py>,
{
#[inline]
fn extract(obj: &'source PyAny) -> PyResult<Self> {
fn extract_bound(obj: &Bound<'py, PyAny>) -> PyResult<Self> {
if let Ok(l) = obj.extract::<L>() {
Ok(Either::Left(l))
} else if let Ok(r) = obj.extract::<R>() {
Expand Down
Loading
Loading