Revert "Merge pull request #3578 from davidhewitt/typed-helpers"

This reverts commit 7b07d6d21b, reversing
changes made to 99858236bd.
This commit is contained in:
David Hewitt 2024-02-03 20:56:23 +00:00
parent d8c5e7943c
commit 76d1b34cd5
19 changed files with 39 additions and 77 deletions

View File

@ -103,7 +103,7 @@ given signatures should be interpreted as follows:
match op { match op {
CompareOp::Eq => (self.0 == other.0).into_py(py), CompareOp::Eq => (self.0 == other.0).into_py(py),
CompareOp::Ne => (self.0 != other.0).into_py(py), CompareOp::Ne => (self.0 != other.0).into_py(py),
_ => py.NotImplemented().into(), _ => py.NotImplemented(),
} }
} }
} }

View File

@ -82,39 +82,6 @@ Python::with_gil(|py| {
# } # }
``` ```
### `py.None()`, `py.NotImplemented()` and `py.Ellipsis()` now return typed singletons
Previously `py.None()`, `py.NotImplemented()` and `py.Ellipsis()` would return `PyObject`. This had a few downsides:
- `PyObject` does not carry static type information
- `PyObject` takes ownership of a reference to the singletons, adding refcounting performance overhead
- `PyObject` is not gil-bound, meaning follow up method calls might again need `py`, causing repetition
To avoid these downsides, these methods now return typed gil-bound references to the singletons, e.g. `py.None()` returns `&PyNone`. These typed singletons all implement `Into<PyObject>`, so migration is straightforward.
Before:
```rust,compile_fail
# use pyo3::prelude::*;
Python::with_gil(|py| {
let a: PyObject = py.None();
let b: &PyAny = py.None().as_ref(py); // or into_ref(py)
});
```
After:
```rust
# use pyo3::prelude::*;
Python::with_gil(|py| {
// For uses needing a PyObject, add `.into()`
let a: PyObject = py.None().into();
// For uses needing &PyAny, remove `.as_ref(py)`
let b: &PyAny = py.None();
});
```
### `Iter(A)NextOutput` are deprecated ### `Iter(A)NextOutput` are deprecated
The `__next__` and `__anext__` magic methods can now return any type convertible into Python objects directly just like all other `#[pymethods]`. The `IterNextOutput` used by `__next__` and `IterANextOutput` used by `__anext__` are subsequently deprecated. Most importantly, this change allows returning an awaitable from `__anext__` without non-sensically wrapping it into `Yield` or `Some`. Only the return types `Option<T>` and `Result<Option<T>, E>` are still handled in a special manner where `Some(val)` yields `val` and `None` stops iteration. The `__next__` and `__anext__` magic methods can now return any type convertible into Python objects directly just like all other `#[pymethods]`. The `IterNextOutput` used by `__next__` and `IterANextOutput` used by `__anext__` are subsequently deprecated. Most importantly, this change allows returning an awaitable from `__anext__` without non-sensically wrapping it into `Yield` or `Some`. Only the return types `Option<T>` and `Result<Option<T>, E>` are still handled in a special manner where `Some(val)` yields `val` and `None` stops iteration.
@ -433,7 +400,7 @@ fn raise_err() -> anyhow::Result<()> {
Err(PyValueError::new_err("original error message").into()) Err(PyValueError::new_err("original error message").into())
} }
# fn main() { fn main() {
Python::with_gil(|py| { Python::with_gil(|py| {
let rs_func = wrap_pyfunction!(raise_err, py).unwrap(); let rs_func = wrap_pyfunction!(raise_err, py).unwrap();
pyo3::py_run!( pyo3::py_run!(
@ -1212,14 +1179,14 @@ ensure that the Python GIL was held by the current thread). Technically, this wa
To migrate, just pass a `py` argument to any calls to these methods. To migrate, just pass a `py` argument to any calls to these methods.
Before: Before:
```rust,ignore ```rust,compile_fail
# pyo3::Python::with_gil(|py| { # pyo3::Python::with_gil(|py| {
py.None().get_refcnt(); py.None().get_refcnt();
# }) # })
``` ```
After: After:
```rust,compile_fail ```rust
# pyo3::Python::with_gil(|py| { # pyo3::Python::with_gil(|py| {
py.None().get_refcnt(py); py.None().get_refcnt(py);
# }) # })

View File

@ -1 +0,0 @@
Change return type of `py.None()`, `py.NotImplemented()`, and `py.Ellipsis()` from `PyObject` to typed singletons (`&PyNone`, `&PyNotImplemented` and `PyEllipsis` respectively).

View File

@ -8,7 +8,7 @@ fn bench_clean_acquire_gil(b: &mut Bencher<'_>) {
} }
fn bench_dirty_acquire_gil(b: &mut Bencher<'_>) { fn bench_dirty_acquire_gil(b: &mut Bencher<'_>) {
let obj: PyObject = Python::with_gil(|py| py.None().into()); let obj = Python::with_gil(|py| py.None());
b.iter_batched( b.iter_batched(
|| { || {
// Clone and drop an object so that the GILPool has work to do. // Clone and drop an object so that the GILPool has work to do.

View File

@ -6,7 +6,7 @@ fn drop_many_objects(b: &mut Bencher<'_>) {
Python::with_gil(|py| { Python::with_gil(|py| {
b.iter(|| { b.iter(|| {
for _ in 0..1000 { for _ in 0..1000 {
std::mem::drop(PyObject::from(py.None())); std::mem::drop(py.None());
} }
}); });
}); });

View File

@ -743,7 +743,7 @@ fn impl_simple_enum(
return Ok((self_val == other.__pyo3__int__()).to_object(py)); return Ok((self_val == other.__pyo3__int__()).to_object(py));
} }
return Ok(::std::convert::Into::into(py.NotImplemented())); return Ok(py.NotImplemented());
} }
_pyo3::basic::CompareOp::Ne => { _pyo3::basic::CompareOp::Ne => {
let self_val = self.__pyo3__int__(); let self_val = self.__pyo3__int__();
@ -754,9 +754,9 @@ fn impl_simple_enum(
return Ok((self_val != other.__pyo3__int__()).to_object(py)); return Ok((self_val != other.__pyo3__int__()).to_object(py));
} }
return Ok(::std::convert::Into::into(py.NotImplemented())); return Ok(py.NotImplemented());
} }
_ => Ok(::std::convert::Into::into(py.NotImplemented())), _ => Ok(py.NotImplemented()),
} }
} }
}; };

View File

@ -37,7 +37,7 @@ impl IterAwaitable {
Ok(v) => Err(PyStopIteration::new_err(v)), Ok(v) => Err(PyStopIteration::new_err(v)),
Err(err) => Err(err), Err(err) => Err(err),
}, },
_ => Ok(py.None().into()), _ => Ok(py.None()),
} }
} }
} }

View File

@ -141,7 +141,7 @@ pub trait ToPyObject {
/// match self { /// match self {
/// Self::Integer(val) => val.into_py(py), /// Self::Integer(val) => val.into_py(py),
/// Self::String(val) => val.into_py(py), /// Self::String(val) => val.into_py(py),
/// Self::None => py.None().into(), /// Self::None => py.None(),
/// } /// }
/// } /// }
/// } /// }
@ -266,7 +266,7 @@ where
{ {
fn to_object(&self, py: Python<'_>) -> PyObject { fn to_object(&self, py: Python<'_>) -> PyObject {
self.as_ref() self.as_ref()
.map_or_else(|| py.None().into(), |val| val.to_object(py)) .map_or_else(|| py.None(), |val| val.to_object(py))
} }
} }
@ -275,7 +275,7 @@ where
T: IntoPy<PyObject>, T: IntoPy<PyObject>,
{ {
fn into_py(self, py: Python<'_>) -> PyObject { fn into_py(self, py: Python<'_>) -> PyObject {
self.map_or_else(|| py.None().into(), |val| val.into_py(py)) self.map_or_else(|| py.None(), |val| val.into_py(py))
} }
} }
@ -647,13 +647,13 @@ mod tests {
assert_eq!(option.as_ptr(), std::ptr::null_mut()); assert_eq!(option.as_ptr(), std::ptr::null_mut());
let none = py.None(); let none = py.None();
option = Some(none.into()); option = Some(none.clone());
let ref_cnt = none.get_refcnt(); let ref_cnt = none.get_refcnt(py);
assert_eq!(option.as_ptr(), none.as_ptr()); assert_eq!(option.as_ptr(), none.as_ptr());
// Ensure ref count not changed by as_ptr call // Ensure ref count not changed by as_ptr call
assert_eq!(none.get_refcnt(), ref_cnt); assert_eq!(none.get_refcnt(py), ref_cnt);
}); });
} }
} }

View File

@ -635,7 +635,7 @@ mod tests {
// Test that if a user tries to convert a python's timezone aware datetime into a naive // Test that if a user tries to convert a python's timezone aware datetime into a naive
// one, the conversion fails. // one, the conversion fails.
Python::with_gil(|py| { Python::with_gil(|py| {
let none = py.None(); let none = py.None().into_ref(py);
assert_eq!( assert_eq!(
none.extract::<Duration>().unwrap_err().to_string(), none.extract::<Duration>().unwrap_err().to_string(),
"TypeError: 'NoneType' object cannot be converted to 'PyDelta'" "TypeError: 'NoneType' object cannot be converted to 'PyDelta'"

View File

@ -219,7 +219,7 @@ impl PyErr {
} else { } else {
// Assume obj is Type[Exception]; let later normalization handle if this // Assume obj is Type[Exception]; let later normalization handle if this
// is not the case // is not the case
PyErrState::lazy(obj, Option::<PyObject>::None) PyErrState::lazy(obj, obj.py().None())
}; };
PyErr::from_state(state) PyErr::from_state(state)

View File

@ -314,15 +314,15 @@ fn test_inc_dec_ref_immortal() {
Python::with_gil(|py| { Python::with_gil(|py| {
let obj = py.None(); let obj = py.None();
let ref_count = obj.get_refcnt(); let ref_count = obj.get_refcnt(py);
let ptr = obj.as_ptr(); let ptr = obj.as_ptr();
unsafe { Py_INCREF(ptr) }; unsafe { Py_INCREF(ptr) };
assert_eq!(obj.get_refcnt(), ref_count); assert_eq!(obj.get_refcnt(py), ref_count);
unsafe { Py_DECREF(ptr) }; unsafe { Py_DECREF(ptr) };
assert_eq!(obj.get_refcnt(), ref_count); assert_eq!(obj.get_refcnt(py), ref_count);
}) })
} }

View File

@ -698,22 +698,22 @@ impl<'py> Python<'py> {
/// Gets the Python builtin value `None`. /// Gets the Python builtin value `None`.
#[allow(non_snake_case)] // the Python keyword starts with uppercase #[allow(non_snake_case)] // the Python keyword starts with uppercase
#[inline] #[inline]
pub fn None(self) -> &'py PyNone { pub fn None(self) -> PyObject {
PyNone::get(self) PyNone::get(self).into()
} }
/// Gets the Python builtin value `Ellipsis`, or `...`. /// Gets the Python builtin value `Ellipsis`, or `...`.
#[allow(non_snake_case)] // the Python keyword starts with uppercase #[allow(non_snake_case)] // the Python keyword starts with uppercase
#[inline] #[inline]
pub fn Ellipsis(self) -> &'py PyEllipsis { pub fn Ellipsis(self) -> PyObject {
PyEllipsis::get(self) PyEllipsis::get(self).into()
} }
/// Gets the Python builtin value `NotImplemented`. /// Gets the Python builtin value `NotImplemented`.
#[allow(non_snake_case)] // the Python keyword starts with uppercase #[allow(non_snake_case)] // the Python keyword starts with uppercase
#[inline] #[inline]
pub fn NotImplemented(self) -> &'py PyNotImplemented { pub fn NotImplemented(self) -> PyObject {
PyNotImplemented::get(self) PyNotImplemented::get(self).into()
} }
/// Gets the running Python interpreter version as a string. /// Gets the running Python interpreter version as a string.

View File

@ -526,7 +526,7 @@ mod tests {
use crate::types::bytearray::PyByteArrayMethods; use crate::types::bytearray::PyByteArrayMethods;
use crate::types::string::PyStringMethods; use crate::types::string::PyStringMethods;
use crate::types::PyByteArray; use crate::types::PyByteArray;
use crate::{exceptions, Bound, PyAny, PyNativeType}; use crate::{exceptions, Bound, PyAny};
use crate::{PyObject, Python}; use crate::{PyObject, Python};
#[test] #[test]
@ -596,7 +596,7 @@ mod tests {
#[test] #[test]
fn test_from_err() { fn test_from_err() {
Python::with_gil(|py| { Python::with_gil(|py| {
if let Err(err) = PyByteArray::from_bound(&py.None().as_borrowed()) { if let Err(err) = PyByteArray::from_bound(py.None().bind(py)) {
assert!(err.is_instance_of::<exceptions::PyTypeError>(py)); assert!(err.is_instance_of::<exceptions::PyTypeError>(py));
} else { } else {
panic!("error"); panic!("error");

View File

@ -486,7 +486,7 @@ impl RichComparisons2 {
match op { match op {
CompareOp::Eq => true.into_py(other.py()), CompareOp::Eq => true.into_py(other.py()),
CompareOp::Ne => false.into_py(other.py()), CompareOp::Ne => false.into_py(other.py()),
_ => other.py().NotImplemented().into(), _ => other.py().NotImplemented(),
} }
} }
} }
@ -556,7 +556,7 @@ mod return_not_implemented {
} }
fn __richcmp__(&self, other: PyRef<'_, Self>, _op: CompareOp) -> PyObject { fn __richcmp__(&self, other: PyRef<'_, Self>, _op: CompareOp) -> PyObject {
other.py().None().into() other.py().None()
} }
fn __add__<'p>(slf: PyRef<'p, Self>, _other: PyRef<'p, Self>) -> PyRef<'p, Self> { fn __add__<'p>(slf: PyRef<'p, Self>, _other: PyRef<'p, Self>) -> PyRef<'p, Self> {

View File

@ -119,7 +119,7 @@ fn test_polymorphic_container_does_not_accept_other_types() {
let setattr = |value: PyObject| p.as_ref(py).setattr("inner", value); let setattr = |value: PyObject| p.as_ref(py).setattr("inner", value);
assert!(setattr(1i32.into_py(py)).is_err()); assert!(setattr(1i32.into_py(py)).is_err());
assert!(setattr(py.None().into()).is_err()); assert!(setattr(py.None()).is_err());
assert!(setattr((1i32, 2i32).into_py(py)).is_err()); assert!(setattr((1i32, 2i32).into_py(py)).is_err());
}); });
} }

View File

@ -118,7 +118,7 @@ fn test_write_unraisable() {
assert!(object.is_none(py)); assert!(object.is_none(py));
let err = PyRuntimeError::new_err("bar"); let err = PyRuntimeError::new_err("bar");
err.write_unraisable(py, Some(py.NotImplemented())); err.write_unraisable(py, Some(py.NotImplemented().as_ref(py)));
let (err, object) = capture.borrow_mut(py).capture.take().unwrap(); let (err, object) = capture.borrow_mut(py).capture.take().unwrap();
assert_eq!(err.to_string(), "RuntimeError: bar"); assert_eq!(err.to_string(), "RuntimeError: bar");

View File

@ -352,7 +352,7 @@ fn test_enum() {
_ => panic!("Expected extracting Foo::TransparentTuple, got {:?}", f), _ => panic!("Expected extracting Foo::TransparentTuple, got {:?}", f),
} }
let none = py.None(); let none = py.None();
let f = Foo::extract(none).expect("Failed to extract Foo from int"); let f = Foo::extract(none.as_ref(py)).expect("Failed to extract Foo from int");
match f { match f {
Foo::TransparentStructVar { a } => assert!(a.is_none()), Foo::TransparentStructVar { a } => assert!(a.is_none()),
_ => panic!("Expected extracting Foo::TransparentStructVar, got {:?}", f), _ => panic!("Expected extracting Foo::TransparentStructVar, got {:?}", f),

View File

@ -89,7 +89,7 @@ impl GcIntegration {
fn __clear__(&mut self) { fn __clear__(&mut self) {
Python::with_gil(|py| { Python::with_gil(|py| {
self.self_ref = py.None().into(); self.self_ref = py.None();
}); });
} }
} }
@ -102,7 +102,7 @@ fn gc_integration() {
let inst = PyCell::new( let inst = PyCell::new(
py, py,
GcIntegration { GcIntegration {
self_ref: py.None().into(), self_ref: py.None(),
dropped: TestDropCall { dropped: TestDropCall {
drop_called: Arc::clone(&drop_called), drop_called: Arc::clone(&drop_called),
}, },
@ -286,9 +286,7 @@ struct PartialTraverse {
impl PartialTraverse { impl PartialTraverse {
fn new(py: Python<'_>) -> Self { fn new(py: Python<'_>) -> Self {
Self { Self { member: py.None() }
member: py.None().into(),
}
} }
} }
@ -324,9 +322,7 @@ struct PanickyTraverse {
impl PanickyTraverse { impl PanickyTraverse {
fn new(py: Python<'_>) -> Self { fn new(py: Python<'_>) -> Self {
Self { Self { member: py.None() }
member: py.None().into(),
}
} }
} }

View File

@ -5,7 +5,7 @@
#[pyo3::pyfunction] #[pyo3::pyfunction]
#[pyo3(name = "identity", signature = (x = None))] #[pyo3(name = "identity", signature = (x = None))]
fn basic_function(py: pyo3::Python<'_>, x: Option<pyo3::PyObject>) -> pyo3::PyObject { fn basic_function(py: pyo3::Python<'_>, x: Option<pyo3::PyObject>) -> pyo3::PyObject {
x.unwrap_or_else(|| py.None().into()) x.unwrap_or_else(|| py.None())
} }
#[pyo3::pymodule] #[pyo3::pymodule]