Skip to content

Commit

Permalink
Merge pull request #3601 from davidhewitt/deprecate-pytryfrom
Browse files Browse the repository at this point in the history
deprecate `PyTryFrom` and `PyTryInto`
  • Loading branch information
davidhewitt committed Dec 20, 2023
2 parents 3583b9a + bc87b7b commit a3c92fa
Show file tree
Hide file tree
Showing 10 changed files with 79 additions and 41 deletions.
1 change: 1 addition & 0 deletions guide/src/migration.md
Expand Up @@ -16,6 +16,7 @@ To migrate, switch all type casts to use `obj.downcast()` instead of `try_from(o
Before:

```rust
# #![allow(deprecated)]
# use pyo3::prelude::*;
# use pyo3::types::{PyInt, PyList};
# fn main() -> PyResult<()> {
Expand Down
1 change: 1 addition & 0 deletions newsfragments/3601.changed.md
@@ -0,0 +1 @@
Deprecate `PyTryFrom` and `PyTryInto` traits in favor of `any.downcast()` via the `PyTypeCheck` and `PyTypeInfo` traits.
100 changes: 63 additions & 37 deletions src/conversion.rs
Expand Up @@ -355,11 +355,20 @@ where
/// If `T` implements `PyTryFrom`, we can convert `&PyAny` to `&T`.
///
/// This trait is similar to `std::convert::TryFrom`
#[deprecated(since = "0.21.0")]
pub trait PyTryFrom<'v>: Sized + PyNativeType {
/// Cast from a concrete Python object type to PyObject.
#[deprecated(
since = "0.21.0",
note = "use `value.downcast::<T>()` instead of `T::try_from(value)`"
)]
fn try_from<V: Into<&'v PyAny>>(value: V) -> Result<&'v Self, PyDowncastError<'v>>;

/// Cast from a concrete Python object type to PyObject. With exact type check.
#[deprecated(
since = "0.21.0",
note = "use `value.downcast_exact::<T>()` instead of `T::try_from_exact(value)`"
)]
fn try_from_exact<V: Into<&'v PyAny>>(value: V) -> Result<&'v Self, PyDowncastError<'v>>;

/// Cast a PyAny to a specific type of PyObject. The caller must
Expand All @@ -368,19 +377,33 @@ pub trait PyTryFrom<'v>: Sized + PyNativeType {
/// # Safety
///
/// Callers must ensure that the type is valid or risk type confusion.
#[deprecated(
since = "0.21.0",
note = "use `value.downcast_unchecked::<T>()` instead of `T::try_from_unchecked(value)`"
)]
unsafe fn try_from_unchecked<V: Into<&'v PyAny>>(value: V) -> &'v Self;
}

/// Trait implemented by Python object types that allow a checked downcast.
/// This trait is similar to `std::convert::TryInto`
#[deprecated(since = "0.21.0")]
pub trait PyTryInto<T>: Sized {
/// Cast from PyObject to a concrete Python object type.
#[deprecated(
since = "0.21.0",
note = "use `value.downcast()` instead of `value.try_into()`"
)]
fn try_into(&self) -> Result<&T, PyDowncastError<'_>>;

/// Cast from PyObject to a concrete Python object type. With exact type check.
#[deprecated(
since = "0.21.0",
note = "use `value.downcast()` instead of `value.try_into_exact()`"
)]
fn try_into_exact(&self) -> Result<&T, PyDowncastError<'_>>;
}

#[allow(deprecated)]
mod implementations {
use super::*;

Expand Down Expand Up @@ -555,47 +578,50 @@ mod test_no_clone {}

#[cfg(test)]
mod tests {
use crate::PyObject;

use super::super::PyTryFrom;
use crate::types::{IntoPyDict, PyAny, PyDict, PyList};
use crate::{Python, ToPyObject};

#[test]
fn test_try_from() {
Python::with_gil(|py| {
let list: &PyAny = vec![3, 6, 5, 4, 7].to_object(py).into_ref(py);
let dict: &PyAny = vec![("reverse", true)].into_py_dict(py).as_ref();

assert!(<PyList as PyTryFrom<'_>>::try_from(list).is_ok());
assert!(<PyDict as PyTryFrom<'_>>::try_from(dict).is_ok());

assert!(<PyAny as PyTryFrom<'_>>::try_from(list).is_ok());
assert!(<PyAny as PyTryFrom<'_>>::try_from(dict).is_ok());
});
}
use crate::{PyObject, Python};

#[allow(deprecated)]
mod deprecated {
use super::super::PyTryFrom;
use crate::types::{IntoPyDict, PyAny, PyDict, PyList};
use crate::{Python, ToPyObject};

#[test]
fn test_try_from() {
Python::with_gil(|py| {
let list: &PyAny = vec![3, 6, 5, 4, 7].to_object(py).into_ref(py);
let dict: &PyAny = vec![("reverse", true)].into_py_dict(py).as_ref();

assert!(<PyList as PyTryFrom<'_>>::try_from(list).is_ok());
assert!(<PyDict as PyTryFrom<'_>>::try_from(dict).is_ok());

assert!(<PyAny as PyTryFrom<'_>>::try_from(list).is_ok());
assert!(<PyAny as PyTryFrom<'_>>::try_from(dict).is_ok());
});
}

#[test]
fn test_try_from_exact() {
Python::with_gil(|py| {
let list: &PyAny = vec![3, 6, 5, 4, 7].to_object(py).into_ref(py);
let dict: &PyAny = vec![("reverse", true)].into_py_dict(py).as_ref();
#[test]
fn test_try_from_exact() {
Python::with_gil(|py| {
let list: &PyAny = vec![3, 6, 5, 4, 7].to_object(py).into_ref(py);
let dict: &PyAny = vec![("reverse", true)].into_py_dict(py).as_ref();

assert!(PyList::try_from_exact(list).is_ok());
assert!(PyDict::try_from_exact(dict).is_ok());
assert!(PyList::try_from_exact(list).is_ok());
assert!(PyDict::try_from_exact(dict).is_ok());

assert!(PyAny::try_from_exact(list).is_err());
assert!(PyAny::try_from_exact(dict).is_err());
});
}
assert!(PyAny::try_from_exact(list).is_err());
assert!(PyAny::try_from_exact(dict).is_err());
});
}

#[test]
fn test_try_from_unchecked() {
Python::with_gil(|py| {
let list = PyList::new(py, [1, 2, 3]);
let val = unsafe { <PyList as PyTryFrom>::try_from_unchecked(list.as_ref()) };
assert!(list.is(val));
});
#[test]
fn test_try_from_unchecked() {
Python::with_gil(|py| {
let list = PyList::new(py, [1, 2, 3]);
let val = unsafe { <PyList as PyTryFrom>::try_from_unchecked(list.as_ref()) };
assert!(list.is(val));
});
}
}

#[test]
Expand Down
4 changes: 3 additions & 1 deletion src/instance.rs
Expand Up @@ -1614,7 +1614,7 @@ a = A()

#[cfg(feature = "macros")]
mod using_macros {
use crate::{PyCell, PyTryInto};
use crate::PyCell;

use super::*;

Expand Down Expand Up @@ -1642,7 +1642,9 @@ a = A()
}

#[test]
#[allow(deprecated)]
fn cell_tryfrom() {
use crate::PyTryInto;
// More detailed tests of the underlying semantics in pycell.rs
Python::with_gil(|py| {
let instance: &PyAny = Py::new(py, SomeClass(0)).unwrap().into_ref(py);
Expand Down
1 change: 1 addition & 0 deletions src/lib.rs
Expand Up @@ -295,6 +295,7 @@
//! [`Ungil`]: crate::marker::Ungil
pub use crate::class::*;
pub use crate::conversion::{AsPyPointer, FromPyObject, FromPyPointer, IntoPy, ToPyObject};
#[allow(deprecated)]
pub use crate::conversion::{PyTryFrom, PyTryInto};
pub use crate::err::{PyDowncastError, PyErr, PyErrArguments, PyResult};
pub use crate::gil::GILPool;
Expand Down
1 change: 1 addition & 0 deletions src/prelude.rs
Expand Up @@ -9,6 +9,7 @@
//! ```

pub use crate::conversion::{FromPyObject, IntoPy, ToPyObject};
#[allow(deprecated)]
pub use crate::conversion::{PyTryFrom, PyTryInto};
pub use crate::err::{PyErr, PyResult};
pub use crate::instance::{Py, PyObject};
Expand Down
1 change: 0 additions & 1 deletion src/type_object.rs
Expand Up @@ -23,7 +23,6 @@ pub trait PySizedLayout<T>: PyLayout<T> + Sized {}
///
/// This is expected to be deprecated in the near future, see <https://github.com/PyO3/pyo3/issues/3382>
///
///
/// # Safety
///
/// - `Py<Self>::as_ref` will hand out references to `Self::AsRefTarget`.
Expand Down
1 change: 1 addition & 0 deletions src/types/iterator.rs
Expand Up @@ -79,6 +79,7 @@ impl PyTypeCheck for PyIterator {
}
}

#[allow(deprecated)]
impl<'v> crate::PyTryFrom<'v> for PyIterator {
fn try_from<V: Into<&'v PyAny>>(value: V) -> Result<&'v PyIterator, PyDowncastError<'v>> {
let value = value.into();
Expand Down
5 changes: 4 additions & 1 deletion src/types/mapping.rs
Expand Up @@ -135,6 +135,7 @@ impl PyTypeCheck for PyMapping {
}
}

#[allow(deprecated)]
impl<'v> crate::PyTryFrom<'v> for PyMapping {
/// Downcasting to `PyMapping` requires the concrete class to be a subclass (or registered
/// subclass) of `collections.abc.Mapping` (from the Python standard library) - i.e.
Expand Down Expand Up @@ -168,7 +169,7 @@ mod tests {
use crate::{
exceptions::PyKeyError,
types::{PyDict, PyTuple},
PyTryFrom, Python,
Python,
};

use super::*;
Expand Down Expand Up @@ -318,7 +319,9 @@ mod tests {
}

#[test]
#[allow(deprecated)]
fn test_mapping_try_from() {
use crate::PyTryFrom;
Python::with_gil(|py| {
let dict = PyDict::new(py);
let _ = <PyMapping as PyTryFrom>::try_from(dict).unwrap();
Expand Down
5 changes: 4 additions & 1 deletion src/types/sequence.rs
Expand Up @@ -549,6 +549,7 @@ impl PyTypeCheck for PySequence {
}
}

#[allow(deprecated)]
impl<'v> crate::PyTryFrom<'v> for PySequence {
/// Downcasting to `PySequence` requires the concrete class to be a subclass (or registered
/// subclass) of `collections.abc.Sequence` (from the Python standard library) - i.e.
Expand Down Expand Up @@ -577,7 +578,7 @@ impl<'v> crate::PyTryFrom<'v> for PySequence {
#[cfg(test)]
mod tests {
use crate::types::{PyList, PySequence, PyTuple};
use crate::{PyObject, PyTryFrom, Python, ToPyObject};
use crate::{PyObject, Python, ToPyObject};

fn get_object() -> PyObject {
// Convenience function for getting a single unique object
Expand Down Expand Up @@ -1078,7 +1079,9 @@ mod tests {
}

#[test]
#[allow(deprecated)]
fn test_seq_try_from() {
use crate::PyTryFrom;
Python::with_gil(|py| {
let list = PyList::empty(py);
let _ = <PySequence as PyTryFrom>::try_from(list).unwrap();
Expand Down

0 comments on commit a3c92fa

Please sign in to comment.