Prefer Py::new to PyCell::new

CHANGELOG.md

@@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 ### Added
 - Add FFI definition `PyObject_AsFileDescriptor` [#938](https://github.com/PyO3/pyo3/pull/938)
 - Add `PyByteArray::data`, `PyByteArray::as_bytes`, and `PyByteArray::as_bytes_mut`. [#967](https://github.com/PyO3/pyo3/pull/967)
+- Add `Py::borrow`, `Py::borrow_mut`, `Py::try_borrow`, and `Py::try_borrow_mut` for accessing `#[pyclass]` values. [#976](https://github.com/PyO3/pyo3/pull/976)
 
 ### Changed
 - Simplify internals of `#[pyo3(get)]` attribute. (Remove the hidden API `GetPropertyValue`.) [#934](https://github.com/PyO3/pyo3/pull/934)

guide/src/class.md

@@ -242,7 +242,7 @@ impl SubSubClass {
 }
 # let gil = Python::acquire_gil();
 # let py = gil.python();
-# let subsub = pyo3::PyCell::new(py, SubSubClass::new()).unwrap();
+# let subsub = Py::new(py, SubSubClass::new()).unwrap();
 # pyo3::py_run!(py, subsub, "assert subsub.method3() == 3000")
 ```
 
@@ -279,7 +279,7 @@ impl DictWithCounter {
 }
 # let gil = Python::acquire_gil();
 # let py = gil.python();
-# let cnt = pyo3::PyCell::new(py, DictWithCounter::new()).unwrap();
+# let cnt = Py::new(py, DictWithCounter::new()).unwrap();
 # pyo3::py_run!(py, cnt, "cnt.set('abc', 10); assert cnt['abc'] == 10")
 ```
 
@@ -862,13 +862,13 @@ impl PyIterProtocol for Container {
         let iter = Iter {
             inner: slf.iter.clone().into_iter(),
         };
-        PyCell::new(slf.py(), iter).map(Into::into)
+        Py::new(slf.py(), iter)
     }
 }
 
 # let gil = Python::acquire_gil();
 # let py = gil.python();
-# let inst = pyo3::PyCell::new(
+# let inst = Py::new(
 #     py,
 #     Container {
 #         iter: vec![1, 2, 3, 4],

guide/src/migration.md

@@ -101,7 +101,7 @@ impl Names {
 }
 # let gil = Python::acquire_gil();
 # let py = gil.python();
-# let names = PyCell::new(py, Names::new()).unwrap();
+# let names = Py::new(py, Names::new()).unwrap();
 # let borrow_mut_err = py.get_type::<pyo3::pycell::PyBorrowMutError>();
 # pyo3::py_run!(py, names borrow_mut_err, r"
 # try:

guide/src/python_from_rust.md

@@ -82,7 +82,7 @@ let userdata = UserData {
     id: 34,
     name: "Yu".to_string(),
 };
-let userdata = PyCell::new(py, userdata).unwrap();
+let userdata = Py::new(py, userdata).unwrap();
 let userdata_as_tuple = (34, "Yu");
 py_run!(py, userdata userdata_as_tuple, r#"
 assert repr(userdata) == "User Yu(id: 34)"

src/instance.rs

@@ -2,10 +2,11 @@
 use crate::err::{PyErr, PyResult};
 use crate::gil;
 use crate::object::PyObject;
+use crate::pycell::{PyBorrowError, PyBorrowMutError, PyCell};
 use crate::type_object::PyBorrowFlagLayout;
 use crate::{
-    ffi, AsPyPointer, FromPyObject, IntoPy, IntoPyPointer, PyAny, PyCell, PyClass,
-    PyClassInitializer, PyRef, PyRefMut, PyTypeInfo, Python, ToPyObject,
+    ffi, AsPyPointer, FromPyObject, IntoPy, IntoPyPointer, PyAny, PyClass, PyClassInitializer,
+    PyRef, PyRefMut, PyTypeInfo, Python, ToPyObject,
 };
 use std::marker::PhantomData;
 use std::mem;
@@ -49,14 +50,13 @@ unsafe impl<T> Send for Py<T> {}
 
 unsafe impl<T> Sync for Py<T> {}
 
-impl<T> Py<T> {
-    /// Create a new instance `Py<T>`.
-    ///
-    /// You can crate [`PyCell::new`](../pycell/struct.PyCell.html#method.new) and `Py::from`,
-    /// but this method can be more efficient.
+impl<T> Py<T>
+where
+    T: PyClass,
+{
+    /// Create a new instance `Py<T>` of a `#[pyclass]` on the Python heap.
     pub fn new(py: Python, value: impl Into<PyClassInitializer<T>>) -> PyResult<Py<T>>
     where
-        T: PyClass,
         T::BaseLayout: PyBorrowFlagLayout<T::BaseType>,
     {
         let initializer = value.into();
@@ -65,6 +65,80 @@ impl<T> Py<T> {
         Ok(ob)
     }
 
+    /// Immutably borrows the value `T`. This borrow lasts untill the returned `PyRef` exists.
+    ///
+    /// # Panics
+    /// Panics if the value is currently mutably borrowed. For a non-panicking variant, use
+    /// [`try_borrow`](#method.try_borrow).
+    pub fn borrow<'py>(&'py self, py: Python<'py>) -> PyRef<'py, T> {
+        self.as_ref(py).borrow()
+    }
+
+    /// Mutably borrows the value `T`. This borrow lasts untill the returned `PyRefMut` exists.
+    ///
+    /// # Panics
+    /// Panics if the value is currently mutably borrowed. For a non-panicking variant, use
+    /// [`try_borrow_mut`](#method.try_borrow_mut).
+    pub fn borrow_mut<'py>(&'py self, py: Python<'py>) -> PyRefMut<'py, T> {
+        self.as_ref(py).borrow_mut()
+    }
+
+    /// Immutably borrows the value `T`, returning an error if the value is currently
+    /// mutably borrowed. This borrow lasts untill the returned `PyRef` exists.
+    ///
+    /// This is the non-panicking variant of [`borrow`](#method.borrow).
+    ///
+    /// # Example
+    /// ```
+    /// # use pyo3::prelude::*;
+    /// #[pyclass]
+    /// struct Class {}
+    /// let gil = Python::acquire_gil();
+    /// let py = gil.python();
+    /// let c = Py::new(py, Class {}).unwrap();
+    /// {
+    ///     let m = c.borrow_mut(py);
+    ///     assert!(c.try_borrow(py).is_err());
+    /// }
+    ///
+    /// {
+    ///     let m = c.borrow(py);
+    ///     assert!(c.try_borrow(py).is_ok());
+    /// }
+    /// ```
+    pub fn try_borrow<'py>(&'py self, py: Python<'py>) -> Result<PyRef<'py, T>, PyBorrowError> {
+        self.as_ref(py).try_borrow()
+    }
+
+    /// Mutably borrows the value `T`, returning an error if the value is currently borrowed.
+    /// This borrow lasts untill the returned `PyRefMut` exists.
+    ///
+    /// This is the non-panicking variant of [`borrow_mut`](#method.borrow_mut).
+    ///
+    /// # Example
+    /// ```
+    /// # use pyo3::prelude::*;
+    /// #[pyclass]
+    /// struct Class {}
+    /// let gil = Python::acquire_gil();
+    /// let py = gil.python();
+    /// let c = Py::new(py, Class {}).unwrap();
+    /// {
+    ///     let m = c.borrow(py);
+    ///     assert!(c.try_borrow_mut(py).is_err());
+    /// }
+    ///
+    /// assert!(c.try_borrow_mut(py).is_ok());
+    /// ```
+    pub fn try_borrow_mut<'py>(
+        &'py self,
+        py: Python<'py>,
+    ) -> Result<PyRefMut<'py, T>, PyBorrowMutError> {
+        self.as_ref(py).try_borrow_mut()
+    }
+}
+
+impl<T> Py<T> {
     /// Creates a `Py<T>` instance for the given FFI pointer.
     ///
     /// This moves ownership over the pointer into the `Py<T>`.

src/lib.rs

@@ -261,7 +261,7 @@ macro_rules! wrap_pymodule {
 /// }
 /// let gil = Python::acquire_gil();
 /// let py = gil.python();
-/// let time = PyCell::new(py, Time {hour: 8, minute: 43, second: 16}).unwrap();
+/// let time = Py::new(py, Time {hour: 8, minute: 43, second: 16}).unwrap();
 /// let time_as_tuple = (8, 43, 16);
 /// py_run!(py, time time_as_tuple, r#"
 /// assert time.hour == 8

src/pycell.rs

@@ -153,7 +153,7 @@ impl<T: PyClass> PyCellInner<T> {
 /// }
 /// # let gil = Python::acquire_gil();
 /// # let py = gil.python();
-/// # let counter = PyCell::new(py, Counter::default()).unwrap();
+/// # let counter = Py::new(py, Counter::default()).unwrap();
 /// # pyo3::py_run!(py, counter, "assert counter.increment('cat') == 1");
 /// ```
 #[repr(C)]
@@ -166,8 +166,11 @@ pub struct PyCell<T: PyClass> {
 unsafe impl<T: PyClass> PyNativeType for PyCell<T> {}
 
 impl<T: PyClass> PyCell<T> {
-    /// Make new `PyCell` on the Python heap and returns the reference of it.
+    /// Make a new `PyCell` on the Python heap and return the reference to it.
     ///
+    /// In cases where the value in the cell does not need to be accessed immediately after
+    /// creation, consider [`Py::new`](../instance/struct.Py.html#method.new) as a more efficient
+    /// alternative.
     pub fn new(py: Python, value: impl Into<PyClassInitializer<T>>) -> PyResult<&Self>
     where
         T::BaseLayout: PyBorrowFlagLayout<T::BaseType>,
@@ -445,7 +448,7 @@ impl<T: PyClass + fmt::Debug> fmt::Debug for PyCell<T> {
 /// }
 /// # let gil = Python::acquire_gil();
 /// # let py = gil.python();
-/// # let sub = PyCell::new(py, Child::new()).unwrap();
+/// # let sub = Py::new(py, Child::new()).unwrap();
 /// # pyo3::py_run!(py, sub, "assert sub.format() == 'Caterpillar(base: Butterfly, cnt: 3)'");
 /// ```
 pub struct PyRef<'p, T: PyClass> {
@@ -509,7 +512,7 @@ where
     /// }
     /// # let gil = Python::acquire_gil();
     /// # let py = gil.python();
-    /// # let sub = PyCell::new(py, Sub::new()).unwrap();
+    /// # let sub = Py::new(py, Sub::new()).unwrap();
     /// # pyo3::py_run!(py, sub, "assert sub.name() == 'base1 base2 sub'")
     /// ```
     pub fn into_super(self) -> PyRef<'p, U> {

src/pyclass.rs

@@ -73,7 +73,11 @@ pub unsafe fn tp_free_fallback(obj: *mut ffi::PyObject) {
 /// The `#[pyclass]` attribute automatically implements this trait for your Rust struct,
 /// so you don't have to use this trait directly.
 pub trait PyClass:
-    PyTypeInfo<Layout = PyCell<Self>> + Sized + PyClassAlloc + PyMethods + Send
+    PyTypeInfo<Layout = PyCell<Self>, AsRefTarget = PyCell<Self>>
+    + Sized
+    + PyClassAlloc
+    + PyMethods
+    + Send
 {
     /// Specify this class has `#[pyclass(dict)]` or not.
     type Dict: PyClassDict;

tests/test_arithmetics.rs

@@ -47,7 +47,7 @@ fn unary_arithmetic() {
     let gil = Python::acquire_gil();
     let py = gil.python();
 
-    let c = PyCell::new(py, UnaryArithmetic::new(2.7)).unwrap();
+    let c = Py::new(py, UnaryArithmetic::new(2.7)).unwrap();
     py_run!(py, c, "assert repr(-c) == 'UA(-2.7)'");
     py_run!(py, c, "assert repr(+c) == 'UA(2.7)'");
     py_run!(py, c, "assert repr(abs(c)) == 'UA(2.7)'");
@@ -130,7 +130,7 @@ fn inplace_operations() {
     let gil = Python::acquire_gil();
     let py = gil.python();
     let init = |value, code| {
-        let c = PyCell::new(py, InPlaceOperations { value }).unwrap();
+        let c = Py::new(py, InPlaceOperations { value }).unwrap();
         py_run!(py, c, code);
     };
 
@@ -193,7 +193,7 @@ fn binary_arithmetic() {
     let gil = Python::acquire_gil();
     let py = gil.python();
 
-    let c = PyCell::new(py, BinaryArithmetic {}).unwrap();
+    let c = Py::new(py, BinaryArithmetic {}).unwrap();
     py_run!(py, c, "assert c + c == 'BA + BA'");
     py_run!(py, c, "assert c.__add__(c) == 'BA + BA'");
     py_run!(py, c, "assert c + 1 == 'BA + 1'");
@@ -266,7 +266,7 @@ fn rhs_arithmetic() {
     let gil = Python::acquire_gil();
     let py = gil.python();
 
-    let c = PyCell::new(py, RhsArithmetic {}).unwrap();
+    let c = Py::new(py, RhsArithmetic {}).unwrap();
     py_run!(py, c, "assert c.__radd__(1) == '1 + RA'");
     py_run!(py, c, "assert 1 + c == '1 + RA'");
     py_run!(py, c, "assert c.__rsub__(1) == '1 - RA'");
@@ -329,7 +329,7 @@ fn lhs_override_rhs() {
     let gil = Python::acquire_gil();
     let py = gil.python();
 
-    let c = PyCell::new(py, LhsAndRhsArithmetic {}).unwrap();
+    let c = Py::new(py, LhsAndRhsArithmetic {}).unwrap();
     // Not overrided
     py_run!(py, c, "assert c.__radd__(1) == '1 + RA'");
     py_run!(py, c, "assert c.__rsub__(1) == '1 - RA'");
@@ -385,7 +385,7 @@ fn rich_comparisons() {
     let gil = Python::acquire_gil();
     let py = gil.python();
 
-    let c = PyCell::new(py, RichComparisons {}).unwrap();
+    let c = Py::new(py, RichComparisons {}).unwrap();
     py_run!(py, c, "assert (c < c) == 'RC < RC'");
     py_run!(py, c, "assert (c < 1) == 'RC < 1'");
     py_run!(py, c, "assert (1 < c) == 'RC > 1'");
@@ -411,7 +411,7 @@ fn rich_comparisons_python_3_type_error() {
     let gil = Python::acquire_gil();
     let py = gil.python();
 
-    let c2 = PyCell::new(py, RichComparisons2 {}).unwrap();
+    let c2 = Py::new(py, RichComparisons2 {}).unwrap();
     py_expect_exception!(py, c2, "c2 < c2", TypeError);
     py_expect_exception!(py, c2, "c2 < 1", TypeError);
     py_expect_exception!(py, c2, "1 < c2", TypeError);

tests/test_class_conversion.rs

@@ -64,14 +64,13 @@ fn test_polymorphic_container_stores_base_class() {
     let gil = Python::acquire_gil();
     let py = gil.python();
 
-    let p = PyCell::new(
+    let p = Py::new(
         py,
         PolymorphicContainer {
             inner: Py::new(py, BaseClass::default()).unwrap(),
         },
     )
-    .unwrap()
-    .to_object(py);
+    .unwrap();
 
     py_assert!(py, p, "p.inner.foo() == 'BaseClass'");
 }
@@ -81,19 +80,18 @@ fn test_polymorphic_container_stores_sub_class() {
     let gil = Python::acquire_gil();
     let py = gil.python();
 
-    let p = PyCell::new(
+    let p = Py::new(
         py,
         PolymorphicContainer {
             inner: Py::new(py, BaseClass::default()).unwrap(),
         },
     )
-    .unwrap()
-    .to_object(py);
+    .unwrap();
 
     p.as_ref(py)
         .setattr(
             "inner",
-            PyCell::new(
+            Py::new(
                 py,
                 PyClassInitializer::from(BaseClass::default()).add_subclass(SubClass {}),
             )
@@ -109,14 +107,13 @@ fn test_polymorphic_container_does_not_accept_other_types() {
     let gil = Python::acquire_gil();
     let py = gil.python();
 
-    let p = PyCell::new(
+    let p = Py::new(
         py,
         PolymorphicContainer {
             inner: Py::new(py, BaseClass::default()).unwrap(),
         },
     )
-    .unwrap()
-    .to_object(py);
+    .unwrap();
 
     let setattr = |value: PyObject| p.as_ref(py).setattr("inner", value);