From 10804b0d650a4ad7cbe93c48b7fd4959a692500d Mon Sep 17 00:00:00 2001 From: Rico Hageman Date: Wed, 23 Feb 2022 23:56:35 +0100 Subject: [PATCH 1/6] Support from_py_with on struct tuples --- pyo3-macros-backend/src/frompyobject.rs | 50 ++++++++++++++++++------- pyo3-macros-backend/src/pyclass.rs | 2 +- tests/test_frompyobject.rs | 20 ++++++++++ 3 files changed, 57 insertions(+), 15 deletions(-) diff --git a/pyo3-macros-backend/src/frompyobject.rs b/pyo3-macros-backend/src/frompyobject.rs index ef72c069de7..f6763f7b7d1 100644 --- a/pyo3-macros-backend/src/frompyobject.rs +++ b/pyo3-macros-backend/src/frompyobject.rs @@ -106,8 +106,8 @@ enum ContainerType<'a> { StructNewtype(&'a Ident), /// Tuple struct, e.g. `struct Foo(String)`. /// - /// Fields are extracted from a tuple. - Tuple(usize), + /// Fields are extracted from a tuple where the variant contains the list of extraction calls. + Tuple(Vec), /// Tuple newtype, e.g. `#[transparent] struct Foo(String)` /// /// The wrapped field is directly extracted from the object. @@ -149,7 +149,15 @@ impl<'a> Container<'a> { (Fields::Unnamed(_), true) => ContainerType::TupleNewtype, (Fields::Unnamed(unnamed), false) => match unnamed.unnamed.len() { 1 => ContainerType::TupleNewtype, - len => ContainerType::Tuple(len), + _ => { + let mut fields = Vec::new(); + for field in unnamed.unnamed.iter() { + let attrs = FieldPyO3Attributes::from_attrs(&field.attrs)?; + fields.push(attrs) + } + + ContainerType::Tuple(fields) + } }, (Fields::Named(named), true) => { let field = named @@ -196,7 +204,7 @@ impl<'a> Container<'a> { match &self.ty { ContainerType::StructNewtype(ident) => self.build_newtype_struct(Some(ident)), ContainerType::TupleNewtype => self.build_newtype_struct(None), - ContainerType::Tuple(len) => self.build_tuple_struct(*len), + ContainerType::Tuple(tups) => self.build_tuple_struct(tups), ContainerType::Struct(tups) => self.build_struct(tups), } } @@ -233,19 +241,33 @@ impl<'a> Container<'a> { } } - fn build_tuple_struct(&self, len: usize) -> TokenStream { + fn build_tuple_struct(&self, tups: &[FieldPyO3Attributes]) -> TokenStream { let self_ty = &self.path; let mut fields: Punctuated = Punctuated::new(); - for i in 0..len { - let error_msg = format!("failed to extract field {}.{}", quote!(#self_ty), i); - fields.push(quote!( - s.get_item(#i).and_then(_pyo3::types::PyAny::extract).map_err(|inner| { - let py = _pyo3::PyNativeType::py(obj); - let new_err = _pyo3::exceptions::PyTypeError::new_err(#error_msg); - new_err.set_cause(py, ::std::option::Option::Some(inner)); - new_err - })?)); + for (index, attrs) in tups.iter().enumerate() { + let error_msg = format!("failed to extract field {}.{}", quote!(#self_ty), index); + + let extractor = match &attrs.from_py_with { + None => quote!( + obj.get_item(#index)?.extract().map_err(|inner| { + let py = _pyo3::PyNativeType::py(obj); + let new_err = _pyo3::exceptions::PyTypeError::new_err(#error_msg); + new_err.set_cause(py, ::std::option::Option::Some(inner)); + new_err + })?), + Some(FromPyWithAttribute(expr_path)) => quote! ( + #expr_path(obj.get_item(#index)?).map_err(|inner| { + let py = _pyo3::PyNativeType::py(obj); + let new_err = _pyo3::exceptions::PyTypeError::new_err(#error_msg); + new_err.set_cause(py, ::std::option::Option::Some(inner)); + new_err + })? + ), + }; + + fields.push(quote!(#extractor)); } + let len = tups.len(); let msg = if self.is_enum_variant { quote!(::std::format!( "expected tuple of length {}, but got length {}", diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs index ff0085a8fc3..17dc234b58a 100644 --- a/pyo3-macros-backend/src/pyclass.rs +++ b/pyo3-macros-backend/src/pyclass.rs @@ -71,7 +71,7 @@ impl PyClassArgs { } } - /// Adda single expression from the comma separated list in the attribute, which is + /// Add a single expression from the comma separated list in the attribute, which is /// either a single word or an assignment expression fn add_expr(&mut self, expr: &Expr) -> Result<()> { match expr { diff --git a/tests/test_frompyobject.rs b/tests/test_frompyobject.rs index 17428cedde0..57ecf1d6c55 100644 --- a/tests/test_frompyobject.rs +++ b/tests/test_frompyobject.rs @@ -481,3 +481,23 @@ fn test_from_py_with() { assert_eq!(zap.some_object_length, 3usize); }); } + +#[derive(Debug, FromPyObject)] +pub struct ZapTuple( + #[pyo3(item)] String, + #[pyo3(from_py_with = "PyAny::len")] usize, +); + +#[test] +fn test_from_py_with_tuple_struct() { + Python::with_gil(|py| { + let py_zap = py + .eval(r#"("whatever", [1, 2, 3])"#, None, None) + .expect("failed to create dict"); + + let zap = ZapTuple::extract(py_zap).unwrap(); + + assert_eq!(zap.0, "whatever"); + assert_eq!(zap.1, 3usize); + }); +} From 84a763da14a5368fe09e23dcba487e4a6f7dc58e Mon Sep 17 00:00:00 2001 From: Rico Hageman Date: Thu, 24 Feb 2022 00:20:00 +0100 Subject: [PATCH 2/6] Add test to ensure support for from_py_with for enums --- tests/test_frompyobject.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/test_frompyobject.rs b/tests/test_frompyobject.rs index 57ecf1d6c55..152466cc897 100644 --- a/tests/test_frompyobject.rs +++ b/tests/test_frompyobject.rs @@ -501,3 +501,23 @@ fn test_from_py_with_tuple_struct() { assert_eq!(zap.1, 3usize); }); } + +#[derive(Debug, FromPyObject, PartialEq)] +pub enum ZapEnum { + Zip(#[pyo3(from_py_with = "PyAny::len")] usize), + Zap(String, #[pyo3(from_py_with = "PyAny::len")] usize), +} + +#[test] +fn test_from_py_with_enum() { + Python::with_gil(|py| { + let py_zap = py + .eval(r#"("whatever", [1, 2, 3])"#, None, None) + .expect("failed to create dict"); + + let zap = ZapEnum::extract(py_zap).unwrap(); + let expected_zap = ZapEnum::Zap(String::from("whatever"), 3usize); + + assert_eq!(zap, expected_zap); + }); +} From 1839fc208b3ad338bc56c010e67457a9e00ada9f Mon Sep 17 00:00:00 2001 From: Rico Hageman Date: Thu, 24 Feb 2022 00:23:07 +0100 Subject: [PATCH 3/6] Add CHANGELOG.md entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e3a9e4e56e..68897e3deb1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add `PyAny::contains` method (`in` operator for `PyAny`). [#2115](https://github.com/PyO3/pyo3/pull/2115) - Add `PyMapping::contains` method (`in` operator for `PyMapping`). [#2133](https://github.com/PyO3/pyo3/pull/2133) - Add garbage collection magic methods `__traverse__` and `__clear__` to `#[pymethods]`. [#2159](https://github.com/PyO3/pyo3/pull/2159) +- Add support for `from_py_with` on struct tuples and enums to override the default from-Python conversion. [#2181](https://github.com/PyO3/pyo3/pull/2181) ### Changed From 76233e1924116b8e8544b43532c91c00dea43ab3 Mon Sep 17 00:00:00 2001 From: Rico Hageman Date: Thu, 24 Feb 2022 11:27:53 +0100 Subject: [PATCH 4/6] Address pull request comments --- pyo3-macros-backend/src/frompyobject.rs | 37 +++++++++++++------------ tests/test_frompyobject.rs | 4 +-- 2 files changed, 21 insertions(+), 20 deletions(-) diff --git a/pyo3-macros-backend/src/frompyobject.rs b/pyo3-macros-backend/src/frompyobject.rs index f6763f7b7d1..f262df3ecbd 100644 --- a/pyo3-macros-backend/src/frompyobject.rs +++ b/pyo3-macros-backend/src/frompyobject.rs @@ -106,7 +106,8 @@ enum ContainerType<'a> { StructNewtype(&'a Ident), /// Tuple struct, e.g. `struct Foo(String)`. /// - /// Fields are extracted from a tuple where the variant contains the list of extraction calls. + /// Variant contains a list of conversion methods for each of the fields that are directly + /// extracted from the tuple. Tuple(Vec), /// Tuple newtype, e.g. `#[transparent] struct Foo(String)` /// @@ -150,11 +151,11 @@ impl<'a> Container<'a> { (Fields::Unnamed(unnamed), false) => match unnamed.unnamed.len() { 1 => ContainerType::TupleNewtype, _ => { - let mut fields = Vec::new(); - for field in unnamed.unnamed.iter() { - let attrs = FieldPyO3Attributes::from_attrs(&field.attrs)?; - fields.push(attrs) - } + let fields = unnamed + .unnamed + .iter() + .map(|field| FieldPyO3Attributes::from_attrs(&field.attrs)) + .collect::>>()?; ContainerType::Tuple(fields) } @@ -247,23 +248,23 @@ impl<'a> Container<'a> { for (index, attrs) in tups.iter().enumerate() { let error_msg = format!("failed to extract field {}.{}", quote!(#self_ty), index); - let extractor = match &attrs.from_py_with { + let parsed_item = match &attrs.from_py_with { None => quote!( - obj.get_item(#index)?.extract().map_err(|inner| { + obj.get_item(#index)?.extract() + ), + Some(FromPyWithAttribute(expr_path)) => quote! ( + #expr_path(obj.get_item(#index)?) + ), + }; + + let extractor = quote!( + #parsed_item.map_err(|inner| { let py = _pyo3::PyNativeType::py(obj); let new_err = _pyo3::exceptions::PyTypeError::new_err(#error_msg); new_err.set_cause(py, ::std::option::Option::Some(inner)); new_err - })?), - Some(FromPyWithAttribute(expr_path)) => quote! ( - #expr_path(obj.get_item(#index)?).map_err(|inner| { - let py = _pyo3::PyNativeType::py(obj); - let new_err = _pyo3::exceptions::PyTypeError::new_err(#error_msg); - new_err.set_cause(py, ::std::option::Option::Some(inner)); - new_err - })? - ), - }; + })? + ); fields.push(quote!(#extractor)); } diff --git a/tests/test_frompyobject.rs b/tests/test_frompyobject.rs index 152466cc897..a5aa50e4fe9 100644 --- a/tests/test_frompyobject.rs +++ b/tests/test_frompyobject.rs @@ -493,7 +493,7 @@ fn test_from_py_with_tuple_struct() { Python::with_gil(|py| { let py_zap = py .eval(r#"("whatever", [1, 2, 3])"#, None, None) - .expect("failed to create dict"); + .expect("failed to create tuple"); let zap = ZapTuple::extract(py_zap).unwrap(); @@ -513,7 +513,7 @@ fn test_from_py_with_enum() { Python::with_gil(|py| { let py_zap = py .eval(r#"("whatever", [1, 2, 3])"#, None, None) - .expect("failed to create dict"); + .expect("failed to create tuple"); let zap = ZapEnum::extract(py_zap).unwrap(); let expected_zap = ZapEnum::Zap(String::from("whatever"), 3usize); From fc186cdb3bb08e48844d059f9b69dacaacfe69c1 Mon Sep 17 00:00:00 2001 From: Rico Hageman Date: Thu, 24 Feb 2022 22:20:54 +0100 Subject: [PATCH 5/6] Remove item annotation in tuple struct --- tests/test_frompyobject.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_frompyobject.rs b/tests/test_frompyobject.rs index a5aa50e4fe9..29555d3f82e 100644 --- a/tests/test_frompyobject.rs +++ b/tests/test_frompyobject.rs @@ -484,7 +484,7 @@ fn test_from_py_with() { #[derive(Debug, FromPyObject)] pub struct ZapTuple( - #[pyo3(item)] String, + String, #[pyo3(from_py_with = "PyAny::len")] usize, ); From ed698c4b43641817aed7789f7b0281e215ee2719 Mon Sep 17 00:00:00 2001 From: Rico Hageman Date: Thu, 24 Feb 2022 22:21:55 +0100 Subject: [PATCH 6/6] Add documentation related to from_py_with for the FromPyObject trait --- guide/src/conversions/traits.md | 4 ++++ tests/test_frompyobject.rs | 5 +---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/guide/src/conversions/traits.md b/guide/src/conversions/traits.md index 4de449e365b..63fa81d2e70 100644 --- a/guide/src/conversions/traits.md +++ b/guide/src/conversions/traits.md @@ -442,6 +442,10 @@ If the input is neither a string nor an integer, the error message will be: - `pyo3(item)`, `pyo3(item("key"))` - retrieve the field from a mapping, possibly with the custom key specified as an argument. - can be any literal that implements `ToBorrowedObject` +- `pyo3(from_py_with = "...")` + - apply a custom function to convert the field from Python the desired Rust type. + - the argument must be the name of the function as a string. + - the function signature must be `fn(&PyAny) -> PyResult` where `T` is the Rust type of the argument. ### `IntoPy` diff --git a/tests/test_frompyobject.rs b/tests/test_frompyobject.rs index 29555d3f82e..41557f36312 100644 --- a/tests/test_frompyobject.rs +++ b/tests/test_frompyobject.rs @@ -483,10 +483,7 @@ fn test_from_py_with() { } #[derive(Debug, FromPyObject)] -pub struct ZapTuple( - String, - #[pyo3(from_py_with = "PyAny::len")] usize, -); +pub struct ZapTuple(String, #[pyo3(from_py_with = "PyAny::len")] usize); #[test] fn test_from_py_with_tuple_struct() {