Merge pull request #934 from davidhewitt/get-set-cleanups

Remove GetPropertyValue & improve property docs

CHANGELOG.md

@@ -8,6 +8,9 @@ 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)
 
+### Changed
+- Simplify internals of `#[pyo3(get)]` attribute. (Remove the hidden API `GetPropertyValue`.) [#934](https://github.com/PyO3/pyo3/pull/934)
+
 ### Removed
 - Remove `ManagedPyRef` (unused, and needs specialization) [#930](https://github.com/PyO3/pyo3/pull/930)
 

guide/src/class.md

@@ -309,8 +309,38 @@ impl SubClass {
 
 ## Object properties
 
-Property descriptor methods can be defined in a `#[pymethods]` `impl` block only and have to be
-annotated with `#[getter]` and `#[setter]` attributes. For example:
+PyO3 supports two ways to add properties to your `#[pyclass]`:
+- For simple fields with no side effects, a `#[pyo3(get, set)]` attribute can be added directly to the field definition in the `#[pyclass]`.
+- For properties which require computation you can define `#[getter]` and `#[setter]` functions in the `#[pymethods]` block.
+
+We'll cover each of these in the following sections.
+
+### Object properties using `#[pyo3(get, set)]`
+
+For simple cases where a member variable is just read and written with no side effects, you can declare getters and setters in your `#[pyclass]` field definition using the `pyo3` attribute, like in the example below:
+
+```rust
+# use pyo3::prelude::*;
+#[pyclass]
+struct MyClass {
+    #[pyo3(get, set)]
+    num: i32
+}
+```
+
+The above would make the `num` property available for reading and writing from Python code as `self.num`.
+
+Properties can be readonly or writeonly by using just `#[pyo3(get)]` or `#[pyo3(set)]` respectively.
+
+To use these annotations, your field type must implement some conversion traits:
+- For `get` the field type must implement both `IntoPy<PyObject>` and `Clone`.
+- For `set` the field type must implement `FromPyObject`.
+
+### Object properties using `#[getter]` and `#[setter]`
+
+For cases which don't satisfy the `#[pyo3(get, set)]` trait requirements, or need side effects, descriptor methods can be defined in a `#[pymethods]` `impl` block.
+
+This is done using the `#[getter]` and `#[setter]` attributes, like in the example below:
 
 ```rust
 # use pyo3::prelude::*;
@@ -386,20 +416,6 @@ impl MyClass {
 
 In this case, the property `number` is defined and available from Python code as `self.number`.
 
-For simple cases where a member variable is just read and written with no side effects, you
-can also declare getters and setters in your Rust struct field definition, for example:
-
-```rust
-# use pyo3::prelude::*;
-#[pyclass]
-struct MyClass {
-    #[pyo3(get, set)]
-    num: i32
-}
-```
-
-Then it is available from Python code as `self.num`.
-
 ## Instance methods
 
 To define a Python compatible method, an `impl` block for your struct has to be annotated with the

pyo3-derive-backend/src/pymethod.rs

@@ -310,8 +310,7 @@ pub(crate) fn impl_wrap_getter(
             (
                 name.unraw(),
                 quote!({
-                    use pyo3::derive_utils::GetPropertyValue;
-                    (&_slf.#name).get_property_value(_py)
+                    _slf.#name.clone()
                 }),
             )
         }

src/derive_utils.rs

@@ -10,7 +10,7 @@ use crate::instance::PyNativeType;
 use crate::pyclass::PyClass;
 use crate::pyclass_init::PyClassInitializer;
 use crate::types::{PyAny, PyDict, PyModule, PyTuple};
-use crate::{ffi, GILPool, IntoPy, Py, PyCell, PyObject, Python};
+use crate::{ffi, GILPool, IntoPy, PyCell, PyObject, Python};
 use std::cell::UnsafeCell;
 
 /// Description of a python parameter; used for `parse_args()`.
@@ -195,32 +195,6 @@ impl<T: PyClass, I: Into<PyClassInitializer<T>>> IntoPyNewResult<T, I> for PyRes
     }
 }
 
-#[doc(hidden)]
-pub trait GetPropertyValue {
-    fn get_property_value(&self, py: Python) -> PyObject;
-}
-
-impl<T> GetPropertyValue for &T
-where
-    T: IntoPy<PyObject> + Clone,
-{
-    fn get_property_value(&self, py: Python) -> PyObject {
-        (*self).clone().into_py(py)
-    }
-}
-
-impl GetPropertyValue for PyObject {
-    fn get_property_value(&self, py: Python) -> PyObject {
-        self.clone_ref(py)
-    }
-}
-
-impl<T> GetPropertyValue for Py<T> {
-    fn get_property_value(&self, py: Python) -> PyObject {
-        self.clone_ref(py).into()
-    }
-}
-
 /// Utilities for basetype
 #[doc(hidden)]
 pub trait PyBaseTypeUtils {

tests/test_class_basics.rs

@@ -141,8 +141,8 @@ fn empty_class_in_module() {
 
 #[pyclass]
 struct ClassWithObjectField {
-    // PyObject has special case for derive_utils::GetPropertyValue,
-    // so this test is making sure it compiles!
+    // It used to be that PyObject was not supported with (get, set)
+    // - this test is just ensuring it compiles.
     #[pyo3(get, set)]
     value: PyObject,
 }

tests/test_sequence.rs

@@ -4,6 +4,10 @@ use pyo3::exceptions::ValueError;
 use pyo3::prelude::*;
 use pyo3::types::{IntoPyDict, PyList};
 
+use pyo3::py_run;
+
+mod common;
+
 #[pyclass]
 struct ByteSequence {
     elements: Vec<u8>,
@@ -193,3 +197,38 @@ fn test_inplace_repeat() {
     run("s = ByteSequence([1, 2]); s *= 3; assert list(s) == [1, 2, 1, 2, 1, 2]");
     err("s = ByteSequence([1, 2); s *= -1");
 }
+
+// Check that #[pyo3(get, set)] works correctly for Vec<PyObject>
+
+#[pyclass]
+struct GenericList {
+    #[pyo3(get, set)]
+    items: Vec<PyObject>,
+}
+
+#[test]
+fn test_generic_list_get() {
+    let gil = Python::acquire_gil();
+    let py = gil.python();
+
+    let list: PyObject = GenericList {
+        items: [1, 2, 3].iter().map(|i| i.to_object(py)).collect(),
+    }
+    .into_py(py);
+
+    py_assert!(py, list, "list.items == [1, 2, 3]");
+}
+
+#[test]
+fn test_generic_list_set() {
+    let gil = Python::acquire_gil();
+    let py = gil.python();
+
+    let list = PyCell::new(py, GenericList { items: vec![] }).unwrap();
+
+    py_run!(py, list, "list.items = [1, 2, 3]");
+    assert_eq!(
+        list.borrow().items,
+        vec![1.to_object(py), 2.to_object(py), 3.to_object(py)]
+    );
+}