From 0cd69e4409fb55e5d65dd9aaa4bef9bfa71d985f Mon Sep 17 00:00:00 2001 From: Michael Gilbert Date: Wed, 22 May 2024 13:05:18 -0700 Subject: [PATCH 01/24] feat: Add 'ord' option for PyClass and corresponding tests Updated the macros back-end to include 'ord' as an option for PyClass allowing for Python-style ordering comparison of enum variants. Additionally, test cases to verify the proper functioning of this new feature have been introduced. --- pyo3-macros-backend/src/attributes.rs | 1 + pyo3-macros-backend/src/pyclass.rs | 69 ++++++++++++++++++++++++++- tests/test_enum.rs | 31 ++++++++++++ 3 files changed, 100 insertions(+), 1 deletion(-) diff --git a/pyo3-macros-backend/src/attributes.rs b/pyo3-macros-backend/src/attributes.rs index d9c805aa3fa..69f3c40f07f 100644 --- a/pyo3-macros-backend/src/attributes.rs +++ b/pyo3-macros-backend/src/attributes.rs @@ -36,6 +36,7 @@ pub mod kw { syn::custom_keyword!(transparent); syn::custom_keyword!(unsendable); syn::custom_keyword!(weakref); + syn::custom_keyword!(ord); } #[derive(Clone, Debug)] diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs index 47c52c84518..4670044b873 100644 --- a/pyo3-macros-backend/src/pyclass.rs +++ b/pyo3-macros-backend/src/pyclass.rs @@ -72,6 +72,7 @@ pub struct PyClassPyO3Options { pub subclass: Option, pub unsendable: Option, pub weakref: Option, + pub ord: Option } enum PyClassPyO3Option { @@ -90,6 +91,7 @@ enum PyClassPyO3Option { Subclass(kw::subclass), Unsendable(kw::unsendable), Weakref(kw::weakref), + Ord(kw::ord), } impl Parse for PyClassPyO3Option { @@ -125,6 +127,8 @@ impl Parse for PyClassPyO3Option { input.parse().map(PyClassPyO3Option::Unsendable) } else if lookahead.peek(attributes::kw::weakref) { input.parse().map(PyClassPyO3Option::Weakref) + } else if lookahead.peek(attributes::kw::ord) { + input.parse().map(PyClassPyO3Option::Ord) } else { Err(lookahead.error()) } @@ -179,6 +183,7 @@ impl PyClassPyO3Options { PyClassPyO3Option::Subclass(subclass) => set_option!(subclass), PyClassPyO3Option::Unsendable(unsendable) => set_option!(unsendable), PyClassPyO3Option::Weakref(weakref) => set_option!(weakref), + PyClassPyO3Option::Ord(ord) => set_option!(ord), } Ok(()) } @@ -775,6 +780,66 @@ fn impl_simple_enum( (int_impl, int_slot) }; + // Add ordering comparisons if ord option is passed to pyclass + let richcmp_ord = match args.options.ord { + Some(_) => { + quote! { + match op { + #pyo3_path::basic::CompareOp::Gt => { + let self_val = self.__pyo3__int__(); + if let Ok(i) = other.extract::<#repr_type>() { + return Ok((self_val > i).to_object(py)); + } + if let Ok(other) = other.extract::<#pyo3_path::PyRef>() { + return Ok((self_val > other.__pyo3__int__()).to_object(py)); + } + + return Ok(py.NotImplemented()); + } + #pyo3_path::basic::CompareOp::Lt => { + let self_val = self.__pyo3__int__(); + if let Ok(i) = other.extract::<#repr_type>() { + return Ok((self_val < i).to_object(py)); + } + if let Ok(other) = other.extract::<#pyo3_path::PyRef>() { + return Ok((self_val < other.__pyo3__int__()).to_object(py)); + } + + return Ok(py.NotImplemented()); + } + #pyo3_path::basic::CompareOp::Le => { + let self_val = self.__pyo3__int__(); + if let Ok(i) = other.extract::<#repr_type>() { + return Ok((self_val <= i).to_object(py)); + } + if let Ok(other) = other.extract::<#pyo3_path::PyRef>() { + return Ok((self_val <= other.__pyo3__int__()).to_object(py)); + } + + return Ok(py.NotImplemented()); + } + #pyo3_path::basic::CompareOp::Ge => { + let self_val = self.__pyo3__int__(); + if let Ok(i) = other.extract::<#repr_type>() { + return Ok((self_val >= i).to_object(py)); + } + if let Ok(other) = other.extract::<#pyo3_path::PyRef>() { + return Ok((self_val >= other.__pyo3__int__()).to_object(py)); + } + + return Ok(py.NotImplemented()); + } + _ => Ok(py.NotImplemented()), + } + } + } + _ => { + quote! { + Ok(py.NotImplemented()) + } + } + }; + let (default_richcmp, default_richcmp_slot) = { let mut richcmp_impl: syn::ImplItemFn = syn::parse_quote! { fn __pyo3__richcmp__( @@ -809,7 +874,9 @@ fn impl_simple_enum( return Ok(py.NotImplemented()); } - _ => Ok(py.NotImplemented()), + _ => { + #richcmp_ord + }, } } }; diff --git a/tests/test_enum.rs b/tests/test_enum.rs index 63492b8d3cd..e3f40cb0794 100644 --- a/tests/test_enum.rs +++ b/tests/test_enum.rs @@ -13,6 +13,13 @@ pub enum MyEnum { OtherVariant, } +#[pyclass(ord)] +#[derive(Debug, PartialEq, Eq, Clone)] +pub enum MyEnumOrd { + Variant, + OtherVariant, +} + #[test] fn test_enum_class_attr() { Python::with_gil(|py| { @@ -73,6 +80,30 @@ fn test_enum_eq_incomparable() { }) } +#[test] +fn test_enum_ord_comparable_opt_in_only() { + Python::with_gil(|py| { + let var1 = Py::new(py, MyEnum::Variant).unwrap(); + let var2 = Py::new(py, MyEnum::OtherVariant).unwrap(); + // ordering on simple enums if opt in only, thus raising an error below + py_expect_exception!(py, var1 var2, "(var1 > var2) == False", PyTypeError); + }) +} + +#[test] +fn test_enum_ord_comparable() { + Python::with_gil(|py| { + let var1 = Py::new(py, MyEnumOrd::Variant).unwrap(); + let var2 = Py::new(py, MyEnumOrd::OtherVariant).unwrap(); + let var3 = Py::new(py, MyEnumOrd::OtherVariant).unwrap(); + py_assert!(py, var1 var2, "(var1 > var2) == False"); + py_assert!(py, var1 var2, "(var1 < var2) == True"); + py_assert!(py, var1 var2, "(var1 >= var2) == False"); + py_assert!(py, var2 var3, "(var3 >= var2) == True"); + py_assert!(py, var1 var2, "(var1 <= var2) == True"); + }) +} + #[pyclass] enum CustomDiscriminant { One = 1, From 676e1a5180f25ef50299dceba7cda76df05365d2 Mon Sep 17 00:00:00 2001 From: Michael Gilbert Date: Wed, 22 May 2024 13:27:00 -0700 Subject: [PATCH 02/24] update: fix formatting with cargo fmt --- pyo3-macros-backend/src/pyclass.rs | 94 +++++++++++++++--------------- 1 file changed, 47 insertions(+), 47 deletions(-) diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs index 4670044b873..aab0167c1ce 100644 --- a/pyo3-macros-backend/src/pyclass.rs +++ b/pyo3-macros-backend/src/pyclass.rs @@ -72,7 +72,7 @@ pub struct PyClassPyO3Options { pub subclass: Option, pub unsendable: Option, pub weakref: Option, - pub ord: Option + pub ord: Option, } enum PyClassPyO3Option { @@ -782,63 +782,63 @@ fn impl_simple_enum( // Add ordering comparisons if ord option is passed to pyclass let richcmp_ord = match args.options.ord { - Some(_) => { - quote! { - match op { - #pyo3_path::basic::CompareOp::Gt => { - let self_val = self.__pyo3__int__(); - if let Ok(i) = other.extract::<#repr_type>() { - return Ok((self_val > i).to_object(py)); - } - if let Ok(other) = other.extract::<#pyo3_path::PyRef>() { - return Ok((self_val > other.__pyo3__int__()).to_object(py)); - } - - return Ok(py.NotImplemented()); + Some(_) => { + quote! { + match op { + #pyo3_path::basic::CompareOp::Gt => { + let self_val = self.__pyo3__int__(); + if let Ok(i) = other.extract::<#repr_type>() { + return Ok((self_val > i).to_object(py)); + } + if let Ok(other) = other.extract::<#pyo3_path::PyRef>() { + return Ok((self_val > other.__pyo3__int__()).to_object(py)); } - #pyo3_path::basic::CompareOp::Lt => { - let self_val = self.__pyo3__int__(); - if let Ok(i) = other.extract::<#repr_type>() { - return Ok((self_val < i).to_object(py)); - } - if let Ok(other) = other.extract::<#pyo3_path::PyRef>() { - return Ok((self_val < other.__pyo3__int__()).to_object(py)); - } - return Ok(py.NotImplemented()); + return Ok(py.NotImplemented()); + } + #pyo3_path::basic::CompareOp::Lt => { + let self_val = self.__pyo3__int__(); + if let Ok(i) = other.extract::<#repr_type>() { + return Ok((self_val < i).to_object(py)); + } + if let Ok(other) = other.extract::<#pyo3_path::PyRef>() { + return Ok((self_val < other.__pyo3__int__()).to_object(py)); } - #pyo3_path::basic::CompareOp::Le => { - let self_val = self.__pyo3__int__(); - if let Ok(i) = other.extract::<#repr_type>() { - return Ok((self_val <= i).to_object(py)); - } - if let Ok(other) = other.extract::<#pyo3_path::PyRef>() { - return Ok((self_val <= other.__pyo3__int__()).to_object(py)); - } - return Ok(py.NotImplemented()); + return Ok(py.NotImplemented()); + } + #pyo3_path::basic::CompareOp::Le => { + let self_val = self.__pyo3__int__(); + if let Ok(i) = other.extract::<#repr_type>() { + return Ok((self_val <= i).to_object(py)); + } + if let Ok(other) = other.extract::<#pyo3_path::PyRef>() { + return Ok((self_val <= other.__pyo3__int__()).to_object(py)); } - #pyo3_path::basic::CompareOp::Ge => { - let self_val = self.__pyo3__int__(); - if let Ok(i) = other.extract::<#repr_type>() { - return Ok((self_val >= i).to_object(py)); - } - if let Ok(other) = other.extract::<#pyo3_path::PyRef>() { - return Ok((self_val >= other.__pyo3__int__()).to_object(py)); - } - return Ok(py.NotImplemented()); + return Ok(py.NotImplemented()); + } + #pyo3_path::basic::CompareOp::Ge => { + let self_val = self.__pyo3__int__(); + if let Ok(i) = other.extract::<#repr_type>() { + return Ok((self_val >= i).to_object(py)); } - _ => Ok(py.NotImplemented()), + if let Ok(other) = other.extract::<#pyo3_path::PyRef>() { + return Ok((self_val >= other.__pyo3__int__()).to_object(py)); + } + + return Ok(py.NotImplemented()); } + _ => Ok(py.NotImplemented()), } } - _ => { - quote! { - Ok(py.NotImplemented()) - } + } + _ => { + quote! { + Ok(py.NotImplemented()) } - }; + } + }; let (default_richcmp, default_richcmp_slot) = { let mut richcmp_impl: syn::ImplItemFn = syn::parse_quote! { From 21abf2432337ae8a8aecbc6bc55e6602aa2e8727 Mon Sep 17 00:00:00 2001 From: Michael Gilbert Date: Wed, 22 May 2024 13:30:30 -0700 Subject: [PATCH 03/24] update: documented added feature in newsfragments --- newsfragments/4202.added.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 newsfragments/4202.added.md diff --git a/newsfragments/4202.added.md b/newsfragments/4202.added.md new file mode 100644 index 00000000000..e1ebad5b223 --- /dev/null +++ b/newsfragments/4202.added.md @@ -0,0 +1 @@ +Support optional `ord` argument to `#[pyclass]` for simple enums in the form of `#[pyclass(ord)]`. \ No newline at end of file From cea1677bf6ed43233d6ef4e48fd960972d3d4567 Mon Sep 17 00:00:00 2001 From: Michael Gilbert Date: Wed, 22 May 2024 13:46:21 -0700 Subject: [PATCH 04/24] update: updated saved errors for comparison test for invalid pyclass args --- tests/ui/invalid_pyclass_args.stderr | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/ui/invalid_pyclass_args.stderr b/tests/ui/invalid_pyclass_args.stderr index 5b2bd24dd3a..7bdd04b4437 100644 --- a/tests/ui/invalid_pyclass_args.stderr +++ b/tests/ui/invalid_pyclass_args.stderr @@ -1,4 +1,4 @@ -error: expected one of: `crate`, `dict`, `extends`, `freelist`, `frozen`, `get_all`, `mapping`, `module`, `name`, `rename_all`, `sequence`, `set_all`, `subclass`, `unsendable`, `weakref` +error: expected one of: `crate`, `dict`, `extends`, `freelist`, `frozen`, `get_all`, `mapping`, `module`, `name`, `rename_all`, `sequence`, `set_all`, `subclass`, `unsendable`, `weakref`, `ord` --> tests/ui/invalid_pyclass_args.rs:3:11 | 3 | #[pyclass(extend=pyo3::types::PyDict)] @@ -46,7 +46,7 @@ error: expected string literal 24 | #[pyclass(module = my_module)] | ^^^^^^^^^ -error: expected one of: `crate`, `dict`, `extends`, `freelist`, `frozen`, `get_all`, `mapping`, `module`, `name`, `rename_all`, `sequence`, `set_all`, `subclass`, `unsendable`, `weakref` +error: expected one of: `crate`, `dict`, `extends`, `freelist`, `frozen`, `get_all`, `mapping`, `module`, `name`, `rename_all`, `sequence`, `set_all`, `subclass`, `unsendable`, `weakref`, `ord` --> tests/ui/invalid_pyclass_args.rs:27:11 | 27 | #[pyclass(weakrev)] From 01e76bddfbf418357aedf27765a8296f6c7e280d Mon Sep 17 00:00:00 2001 From: Michael Gilbert Date: Wed, 22 May 2024 14:40:49 -0700 Subject: [PATCH 05/24] update: removed nested match arms and extended cases for ordering instead --- pyo3-macros-backend/src/pyclass.rs | 82 ++++++++++++++---------------- 1 file changed, 39 insertions(+), 43 deletions(-) diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs index aab0167c1ce..e1053162add 100644 --- a/pyo3-macros-backend/src/pyclass.rs +++ b/pyo3-macros-backend/src/pyclass.rs @@ -784,59 +784,54 @@ fn impl_simple_enum( let richcmp_ord = match args.options.ord { Some(_) => { quote! { - match op { - #pyo3_path::basic::CompareOp::Gt => { - let self_val = self.__pyo3__int__(); - if let Ok(i) = other.extract::<#repr_type>() { - return Ok((self_val > i).to_object(py)); - } - if let Ok(other) = other.extract::<#pyo3_path::PyRef>() { - return Ok((self_val > other.__pyo3__int__()).to_object(py)); - } - - return Ok(py.NotImplemented()); + #pyo3_path::basic::CompareOp::Gt => { + let self_val = self.__pyo3__int__(); + if let Ok(i) = other.extract::<#repr_type>() { + return Ok((self_val > i).to_object(py)); + } + if let Ok(other) = other.extract::<#pyo3_path::PyRef>() { + return Ok((self_val > other.__pyo3__int__()).to_object(py)); } - #pyo3_path::basic::CompareOp::Lt => { - let self_val = self.__pyo3__int__(); - if let Ok(i) = other.extract::<#repr_type>() { - return Ok((self_val < i).to_object(py)); - } - if let Ok(other) = other.extract::<#pyo3_path::PyRef>() { - return Ok((self_val < other.__pyo3__int__()).to_object(py)); - } - return Ok(py.NotImplemented()); + return Ok(py.NotImplemented()); + } + #pyo3_path::basic::CompareOp::Lt => { + let self_val = self.__pyo3__int__(); + if let Ok(i) = other.extract::<#repr_type>() { + return Ok((self_val < i).to_object(py)); + } + if let Ok(other) = other.extract::<#pyo3_path::PyRef>() { + return Ok((self_val < other.__pyo3__int__()).to_object(py)); } - #pyo3_path::basic::CompareOp::Le => { - let self_val = self.__pyo3__int__(); - if let Ok(i) = other.extract::<#repr_type>() { - return Ok((self_val <= i).to_object(py)); - } - if let Ok(other) = other.extract::<#pyo3_path::PyRef>() { - return Ok((self_val <= other.__pyo3__int__()).to_object(py)); - } - return Ok(py.NotImplemented()); + return Ok(py.NotImplemented()); + } + #pyo3_path::basic::CompareOp::Le => { + let self_val = self.__pyo3__int__(); + if let Ok(i) = other.extract::<#repr_type>() { + return Ok((self_val <= i).to_object(py)); + } + if let Ok(other) = other.extract::<#pyo3_path::PyRef>() { + return Ok((self_val <= other.__pyo3__int__()).to_object(py)); } - #pyo3_path::basic::CompareOp::Ge => { - let self_val = self.__pyo3__int__(); - if let Ok(i) = other.extract::<#repr_type>() { - return Ok((self_val >= i).to_object(py)); - } - if let Ok(other) = other.extract::<#pyo3_path::PyRef>() { - return Ok((self_val >= other.__pyo3__int__()).to_object(py)); - } - return Ok(py.NotImplemented()); + return Ok(py.NotImplemented()); + } + #pyo3_path::basic::CompareOp::Ge => { + let self_val = self.__pyo3__int__(); + if let Ok(i) = other.extract::<#repr_type>() { + return Ok((self_val >= i).to_object(py)); + } + if let Ok(other) = other.extract::<#pyo3_path::PyRef>() { + return Ok((self_val >= other.__pyo3__int__()).to_object(py)); } - _ => Ok(py.NotImplemented()), + + return Ok(py.NotImplemented()); } } } _ => { - quote! { - Ok(py.NotImplemented()) - } + quote! {} } }; @@ -874,8 +869,9 @@ fn impl_simple_enum( return Ok(py.NotImplemented()); } + #richcmp_ord _ => { - #richcmp_ord + Ok(py.NotImplemented()) }, } } From c6349c6651f1247a965a0be5ae1672cd459c7154 Mon Sep 17 00:00:00 2001 From: Michael Gilbert Date: Thu, 23 May 2024 16:55:45 -0700 Subject: [PATCH 06/24] update: alphabetically ordered entries --- pyo3-macros-backend/src/attributes.rs | 2 +- pyo3-macros-backend/src/pyclass.rs | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pyo3-macros-backend/src/attributes.rs b/pyo3-macros-backend/src/attributes.rs index 69f3c40f07f..015ddcd052c 100644 --- a/pyo3-macros-backend/src/attributes.rs +++ b/pyo3-macros-backend/src/attributes.rs @@ -25,6 +25,7 @@ pub mod kw { syn::custom_keyword!(mapping); syn::custom_keyword!(module); syn::custom_keyword!(name); + syn::custom_keyword!(ord); syn::custom_keyword!(pass_module); syn::custom_keyword!(rename_all); syn::custom_keyword!(sequence); @@ -36,7 +37,6 @@ pub mod kw { syn::custom_keyword!(transparent); syn::custom_keyword!(unsendable); syn::custom_keyword!(weakref); - syn::custom_keyword!(ord); } #[derive(Clone, Debug)] diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs index e1053162add..7d9258ddd8d 100644 --- a/pyo3-macros-backend/src/pyclass.rs +++ b/pyo3-macros-backend/src/pyclass.rs @@ -66,13 +66,13 @@ pub struct PyClassPyO3Options { pub mapping: Option, pub module: Option, pub name: Option, + pub ord: Option, pub rename_all: Option, pub sequence: Option, pub set_all: Option, pub subclass: Option, pub unsendable: Option, pub weakref: Option, - pub ord: Option, } enum PyClassPyO3Option { @@ -85,13 +85,13 @@ enum PyClassPyO3Option { Mapping(kw::mapping), Module(ModuleAttribute), Name(NameAttribute), + Ord(kw::ord), RenameAll(RenameAllAttribute), Sequence(kw::sequence), SetAll(kw::set_all), Subclass(kw::subclass), Unsendable(kw::unsendable), Weakref(kw::weakref), - Ord(kw::ord), } impl Parse for PyClassPyO3Option { @@ -115,6 +115,8 @@ impl Parse for PyClassPyO3Option { input.parse().map(PyClassPyO3Option::Module) } else if lookahead.peek(kw::name) { input.parse().map(PyClassPyO3Option::Name) + } else if lookahead.peek(attributes::kw::ord) { + input.parse().map(PyClassPyO3Option::Ord) } else if lookahead.peek(kw::rename_all) { input.parse().map(PyClassPyO3Option::RenameAll) } else if lookahead.peek(attributes::kw::sequence) { @@ -127,8 +129,6 @@ impl Parse for PyClassPyO3Option { input.parse().map(PyClassPyO3Option::Unsendable) } else if lookahead.peek(attributes::kw::weakref) { input.parse().map(PyClassPyO3Option::Weakref) - } else if lookahead.peek(attributes::kw::ord) { - input.parse().map(PyClassPyO3Option::Ord) } else { Err(lookahead.error()) } @@ -177,13 +177,13 @@ impl PyClassPyO3Options { PyClassPyO3Option::Mapping(mapping) => set_option!(mapping), PyClassPyO3Option::Module(module) => set_option!(module), PyClassPyO3Option::Name(name) => set_option!(name), + PyClassPyO3Option::Ord(ord) => set_option!(ord), PyClassPyO3Option::RenameAll(rename_all) => set_option!(rename_all), PyClassPyO3Option::Sequence(sequence) => set_option!(sequence), PyClassPyO3Option::SetAll(set_all) => set_option!(set_all), PyClassPyO3Option::Subclass(subclass) => set_option!(subclass), PyClassPyO3Option::Unsendable(unsendable) => set_option!(unsendable), PyClassPyO3Option::Weakref(weakref) => set_option!(weakref), - PyClassPyO3Option::Ord(ord) => set_option!(ord), } Ok(()) } From ea90363b9c1f5ee80cb0c036f53be95846eb6721 Mon Sep 17 00:00:00 2001 From: Michael Gilbert Date: Thu, 23 May 2024 16:56:12 -0700 Subject: [PATCH 07/24] update: added section to class documentation with example for using ord argument. --- guide/src/class.md | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/guide/src/class.md b/guide/src/class.md index 57a5cf6d467..a63cc30b034 100644 --- a/guide/src/class.md +++ b/guide/src/class.md @@ -1,4 +1,4 @@ -# Python classes +t# Python classes PyO3 exposes a group of attributes powered by Rust's proc macro system for defining Python classes as Rust structs. @@ -1155,6 +1155,30 @@ Python::with_gil(|py| { }) ``` +Ordering of enum variants is optionally added using `#[pyo3(ord)]`. + +```rust +# use pyo3::prelude::*; +#[pyclass(ord)] +enum MyEnum{ + A, + B, + C, +} + +Python::with_gil(|py| { + let cls = py.get_type_bound::(); + let a = Py::new(py, MyEnum::A).unwrap(); + let b = Py::new(py, MyEnum::B).unwrap(); + let c = Py::new(py, MyEnum::C).unwrap(); + pyo3::py_run!(py, cls a b c, r#" + assert (a < b) == True + assert (c <= b) == False + assert (c > a) == True + "#) +}) +``` + You may not use enums as a base class or let enums inherit from other classes. ```rust,compile_fail From d02f65b7450f3e5493e7fa95421ea8f4d3e94ac7 Mon Sep 17 00:00:00 2001 From: Michael Gilbert Date: Thu, 23 May 2024 16:57:32 -0700 Subject: [PATCH 08/24] refactor: reduced duplication of code using closure to process tokens. --- pyo3-macros-backend/src/pyclass.rs | 80 ++++++++++-------------------- 1 file changed, 26 insertions(+), 54 deletions(-) diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs index 7d9258ddd8d..ca40a94eee8 100644 --- a/pyo3-macros-backend/src/pyclass.rs +++ b/pyo3-macros-backend/src/pyclass.rs @@ -780,53 +780,41 @@ fn impl_simple_enum( (int_impl, int_slot) }; + let comparison_arm = |op| { + quote! { + let self_val = self.__pyo3__int__(); + if let Ok(i) = other.extract::<#repr_type>() { + return Ok((self_val #op i).to_object(py)); + } + if let Ok(other) = other.extract::<#pyo3_path::PyRef>() { + return Ok((self_val #op other.__pyo3__int__()).to_object(py)); + } + + return Ok(py.NotImplemented()); + } + }; + let eq_arm = comparison_arm(quote! {==}); + let ne_arm = comparison_arm(quote! {!=}); + // Add ordering comparisons if ord option is passed to pyclass let richcmp_ord = match args.options.ord { Some(_) => { + let gt_arm = comparison_arm(quote! {>}); + let lt_arm = comparison_arm(quote! {<}); + let ge_arm = comparison_arm(quote! {>=}); + let le_arm = comparison_arm(quote! {<=}); quote! { #pyo3_path::basic::CompareOp::Gt => { - let self_val = self.__pyo3__int__(); - if let Ok(i) = other.extract::<#repr_type>() { - return Ok((self_val > i).to_object(py)); - } - if let Ok(other) = other.extract::<#pyo3_path::PyRef>() { - return Ok((self_val > other.__pyo3__int__()).to_object(py)); - } - - return Ok(py.NotImplemented()); + #gt_arm } #pyo3_path::basic::CompareOp::Lt => { - let self_val = self.__pyo3__int__(); - if let Ok(i) = other.extract::<#repr_type>() { - return Ok((self_val < i).to_object(py)); - } - if let Ok(other) = other.extract::<#pyo3_path::PyRef>() { - return Ok((self_val < other.__pyo3__int__()).to_object(py)); - } - - return Ok(py.NotImplemented()); + #lt_arm } #pyo3_path::basic::CompareOp::Le => { - let self_val = self.__pyo3__int__(); - if let Ok(i) = other.extract::<#repr_type>() { - return Ok((self_val <= i).to_object(py)); - } - if let Ok(other) = other.extract::<#pyo3_path::PyRef>() { - return Ok((self_val <= other.__pyo3__int__()).to_object(py)); - } - - return Ok(py.NotImplemented()); + #le_arm } #pyo3_path::basic::CompareOp::Ge => { - let self_val = self.__pyo3__int__(); - if let Ok(i) = other.extract::<#repr_type>() { - return Ok((self_val >= i).to_object(py)); - } - if let Ok(other) = other.extract::<#pyo3_path::PyRef>() { - return Ok((self_val >= other.__pyo3__int__()).to_object(py)); - } - - return Ok(py.NotImplemented()); + #ge_arm } } } @@ -848,26 +836,10 @@ fn impl_simple_enum( use ::core::result::Result::*; match op { #pyo3_path::basic::CompareOp::Eq => { - let self_val = self.__pyo3__int__(); - if let Ok(i) = other.extract::<#repr_type>() { - return Ok((self_val == i).to_object(py)); - } - if let Ok(other) = other.extract::<#pyo3_path::PyRef>() { - return Ok((self_val == other.__pyo3__int__()).to_object(py)); - } - - return Ok(py.NotImplemented()); + #eq_arm } #pyo3_path::basic::CompareOp::Ne => { - let self_val = self.__pyo3__int__(); - if let Ok(i) = other.extract::<#repr_type>() { - return Ok((self_val != i).to_object(py)); - } - if let Ok(other) = other.extract::<#pyo3_path::PyRef>() { - return Ok((self_val != other.__pyo3__int__()).to_object(py)); - } - - return Ok(py.NotImplemented()); + #ne_arm } #richcmp_ord _ => { From 8d2a9742514512dabfcb0a823d6d3eebc04acf37 Mon Sep 17 00:00:00 2001 From: Michael Gilbert Date: Thu, 23 May 2024 16:59:24 -0700 Subject: [PATCH 09/24] update: used ensure_spanned macro to emit compile time errors for uses of ord on complex enums or structs, updated test errors for bad compile cases --- pyo3-macros-backend/src/pyclass.rs | 14 ++++++++++++++ tests/ui/invalid_pyclass_args.rs | 5 +++++ tests/ui/invalid_pyclass_args.stderr | 10 ++++++++-- tests/ui/invalid_pyclass_enum.rs | 6 ++++++ tests/ui/invalid_pyclass_enum.stderr | 6 ++++++ 5 files changed, 39 insertions(+), 2 deletions(-) diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs index ca40a94eee8..f457f6c2f8e 100644 --- a/pyo3-macros-backend/src/pyclass.rs +++ b/pyo3-macros-backend/src/pyclass.rs @@ -214,6 +214,13 @@ pub fn build_py_class( For an explanation, see https://pyo3.rs/latest/class.html#no-generic-parameters" ); + // flag invalid ord option + ensure_spanned!( + args.options.ord.is_none(), + args.options.ord.span() => + "The `ord` option of #[pyclass] is not supported for structs." + ); + let mut field_options: Vec<(&syn::Field, FieldPyO3Options)> = match &mut class.fields { syn::Fields::Named(fields) => fields .named @@ -893,6 +900,13 @@ fn impl_complex_enum( ) -> Result { let Ctx { pyo3_path } = ctx; + // flag invalid ord option + ensure_spanned!( + args.options.ord.is_none(), + args.options.ord.span() => + "The `ord` option of #[pyclass] is not supported for complex enums." + ); + // Need to rig the enum PyClass options let args = { let mut rigged_args = args.clone(); diff --git a/tests/ui/invalid_pyclass_args.rs b/tests/ui/invalid_pyclass_args.rs index fac21db078c..0d67522b8ed 100644 --- a/tests/ui/invalid_pyclass_args.rs +++ b/tests/ui/invalid_pyclass_args.rs @@ -30,4 +30,9 @@ struct InvalidArg {} #[pyclass(mapping, sequence)] struct CannotBeMappingAndSequence {} +#[pyclass(ord)] +struct InvalidOrderedStruct { + inner: i32 +} + fn main() {} diff --git a/tests/ui/invalid_pyclass_args.stderr b/tests/ui/invalid_pyclass_args.stderr index 7bdd04b4437..b544a756ce9 100644 --- a/tests/ui/invalid_pyclass_args.stderr +++ b/tests/ui/invalid_pyclass_args.stderr @@ -1,4 +1,4 @@ -error: expected one of: `crate`, `dict`, `extends`, `freelist`, `frozen`, `get_all`, `mapping`, `module`, `name`, `rename_all`, `sequence`, `set_all`, `subclass`, `unsendable`, `weakref`, `ord` +error: expected one of: `crate`, `dict`, `extends`, `freelist`, `frozen`, `get_all`, `mapping`, `module`, `name`, `ord`, `rename_all`, `sequence`, `set_all`, `subclass`, `unsendable`, `weakref` --> tests/ui/invalid_pyclass_args.rs:3:11 | 3 | #[pyclass(extend=pyo3::types::PyDict)] @@ -46,7 +46,7 @@ error: expected string literal 24 | #[pyclass(module = my_module)] | ^^^^^^^^^ -error: expected one of: `crate`, `dict`, `extends`, `freelist`, `frozen`, `get_all`, `mapping`, `module`, `name`, `rename_all`, `sequence`, `set_all`, `subclass`, `unsendable`, `weakref`, `ord` +error: expected one of: `crate`, `dict`, `extends`, `freelist`, `frozen`, `get_all`, `mapping`, `module`, `name`, `ord`, `rename_all`, `sequence`, `set_all`, `subclass`, `unsendable`, `weakref` --> tests/ui/invalid_pyclass_args.rs:27:11 | 27 | #[pyclass(weakrev)] @@ -57,3 +57,9 @@ error: a `#[pyclass]` cannot be both a `mapping` and a `sequence` | 31 | struct CannotBeMappingAndSequence {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: The `ord` option of #[pyclass] is not supported for structs. + --> tests/ui/invalid_pyclass_args.rs:33:11 + | +33 | #[pyclass(ord)] + | ^^^ diff --git a/tests/ui/invalid_pyclass_enum.rs b/tests/ui/invalid_pyclass_enum.rs index e98010fea32..96679e24a61 100644 --- a/tests/ui/invalid_pyclass_enum.rs +++ b/tests/ui/invalid_pyclass_enum.rs @@ -28,4 +28,10 @@ enum SimpleNoSignature { B, } +#[pyclass(ord)] +enum InvalidOrderedComplexEnum { + VariantA (i32), + VariantB { msg: String } +} + fn main() {} diff --git a/tests/ui/invalid_pyclass_enum.stderr b/tests/ui/invalid_pyclass_enum.stderr index 7e3b6ffa425..df3c664a56d 100644 --- a/tests/ui/invalid_pyclass_enum.stderr +++ b/tests/ui/invalid_pyclass_enum.stderr @@ -29,3 +29,9 @@ error: `constructor` can't be used on a simple enum variant | 26 | #[pyo3(constructor = (a, b))] | ^^^^^^^^^^^ + +error: The `ord` option of #[pyclass] is not supported for complex enums. + --> tests/ui/invalid_pyclass_enum.rs:31:11 + | +31 | #[pyclass(ord)] + | ^^^ From 3fbd4b0ae62506da459a4954b09e2f76c752737e Mon Sep 17 00:00:00 2001 From: Michael Gilbert Date: Fri, 24 May 2024 13:58:07 -0700 Subject: [PATCH 10/24] fix: remove errant character --- guide/src/class.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/guide/src/class.md b/guide/src/class.md index a63cc30b034..b26dda7e591 100644 --- a/guide/src/class.md +++ b/guide/src/class.md @@ -1,4 +1,4 @@ -t# Python classes +# Python classes PyO3 exposes a group of attributes powered by Rust's proc macro system for defining Python classes as Rust structs. From be00a8fc1a0a81042a8d7119da8c86e4d58a9706 Mon Sep 17 00:00:00 2001 From: Michael Gilbert Date: Tue, 28 May 2024 07:12:10 -0700 Subject: [PATCH 11/24] update: added note about PartialOrd being required. --- guide/src/class.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/guide/src/class.md b/guide/src/class.md index b26dda7e591..88eaa579ac8 100644 --- a/guide/src/class.md +++ b/guide/src/class.md @@ -1155,11 +1155,13 @@ Python::with_gil(|py| { }) ``` -Ordering of enum variants is optionally added using `#[pyo3(ord)]`. +Ordering of enum variants is optionally added using `#[pyo3(ord)]`. +*Note: Implementation of the `PartialOrd` trait is required when passing the `ord` argument. If not implemented, a compile time error is raised.* ```rust # use pyo3::prelude::*; #[pyclass(ord)] +#[derive(PartialEq, PartialOrd)] enum MyEnum{ A, B, From 5ed0cf5a155189e8e3602764ef33a709a967aa8f Mon Sep 17 00:00:00 2001 From: Michael Gilbert Date: Tue, 28 May 2024 07:15:19 -0700 Subject: [PATCH 12/24] feat: implemented ordering for structs and complex enums. Retained the equality logic for simple enums until PartialEq is deprecated. --- pyo3-macros-backend/src/pyclass.rs | 213 ++++++++++++++++++----------- 1 file changed, 135 insertions(+), 78 deletions(-) diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs index f457f6c2f8e..eef13710b4a 100644 --- a/pyo3-macros-backend/src/pyclass.rs +++ b/pyo3-macros-backend/src/pyclass.rs @@ -22,7 +22,7 @@ use quote::{format_ident, quote}; use syn::ext::IdentExt; use syn::parse::{Parse, ParseStream}; use syn::punctuated::Punctuated; -use syn::{parse_quote, spanned::Spanned, Result, Token}; +use syn::{parse_quote, spanned::Spanned, ImplItemFn, Result, Token}; /// If the class is derived from a Rust `struct` or `enum`. #[derive(Copy, Clone, Debug, PartialEq, Eq)] @@ -214,13 +214,6 @@ pub fn build_py_class( For an explanation, see https://pyo3.rs/latest/class.html#no-generic-parameters" ); - // flag invalid ord option - ensure_spanned!( - args.options.ord.is_none(), - args.options.ord.span() => - "The `ord` option of #[pyclass] is not supported for structs." - ); - let mut field_options: Vec<(&syn::Field, FieldPyO3Options)> = match &mut class.fields { syn::Fields::Named(fields) => fields .named @@ -362,6 +355,21 @@ fn impl_class( let Ctx { pyo3_path } = ctx; let pytypeinfo_impl = impl_pytypeinfo(cls, args, None, ctx); + // This match case is in place until an `eq` argument is added to the PyClassArgs options and + // will require PartialEq to be implemented. For now, equality is behind the `ord` argument + // for Python class structs, as PartialOrd requires PartialEq. + let (default_richcmp, default_slots) = match args.options.ord { + Some(_) => { + let ty = syn::parse_quote!(#cls); + let (default_richcmp, default_richcmp_slot) = impl_cmp_ops(&args, ctx, &ty, None); + ( + quote! {impl #cls {#default_richcmp}}, + vec![default_richcmp_slot], + ) + } + _ => (quote! {}, vec![]), + }; + let py_class_impl = PyClassImplsBuilder::new( cls, args, @@ -373,13 +381,14 @@ fn impl_class( field_options, ctx, )?, - vec![], + default_slots, ) .doc(doc) .impl_all(ctx)?; Ok(quote! { impl #pyo3_path::types::DerefToPyAny for #cls {} + #default_richcmp #pytypeinfo_impl @@ -728,6 +737,99 @@ fn impl_enum( } } +/// Implements the rich comparison operations (`__eq__`, `__ne__`, `__gt__`, `__lt__`, `__ge__`, `__le__`) +/// for a PyClass. +/// +/// # Arguments +/// +/// * `args` - The PyClassArgs struct that contains information, such as arguments passed to the pyclass macro, about the PyClass. +/// * `ctx` - The Ctx struct that contains the pyo3_path. +/// * `ty` - The syn::Type of the PyClass. +/// * `eq_ne_arms` - An optional tuple containing the custom `__eq__` and `__ne__` arms. If not provided, +/// default comparison arms using the `==` and `!=` operators will be generated. +/// *Once `PartialEq` becomes required for simple enums, this argument will be removed.* +/// +/// # Returns +/// +/// A tuple containing: +/// * `richcmp_impl` - The generated rich comparison implementation as a syn::ImplItemFn. +/// * `richcmp_slot` - The generated protocol slot for `__richcmp__`. +fn impl_cmp_ops( + args: &PyClassArgs, + ctx: &Ctx, + ty: &syn::Type, + eq_ne_arms: Option<(TokenStream, TokenStream)>, +) -> (ImplItemFn, MethodAndSlotDef) { + let Ctx { pyo3_path } = ctx; + let comparison_arm = |op| { + quote! { + let other = &*other.downcast::()?.borrow(); + Ok((self #op other).to_object(py)) + } + }; + + let (eq_arm, ne_arm) = if let Some((eq_arm, ne_arm)) = eq_ne_arms { + (eq_arm, ne_arm) + } else { + (comparison_arm(quote! {==}), comparison_arm(quote! {!=})) + }; + + // Add ordering comparisons if ord option is passed to pyclass + let richcmp_ord = match args.options.ord { + Some(_) => { + let gt_arm = comparison_arm(quote! {>}); + let lt_arm = comparison_arm(quote! {<}); + let ge_arm = comparison_arm(quote! {>=}); + let le_arm = comparison_arm(quote! {<=}); + quote! { + #pyo3_path::basic::CompareOp::Gt => { + #gt_arm + } + #pyo3_path::basic::CompareOp::Lt => { + #lt_arm + } + #pyo3_path::basic::CompareOp::Le => { + #le_arm + } + #pyo3_path::basic::CompareOp::Ge => { + #ge_arm + } + } + } + _ => { + quote! {} + } + }; + + let mut richcmp_impl: syn::ImplItemFn = syn::parse_quote! { + fn __pyo3__richcmp__( + &self, + py: #pyo3_path::Python, + other: &#pyo3_path::Bound<'_, #pyo3_path::PyAny>, + op: #pyo3_path::basic::CompareOp + ) -> #pyo3_path::PyResult<#pyo3_path::PyObject> { + use #pyo3_path::conversion::ToPyObject; + use #pyo3_path::types::PyAnyMethods; + use ::core::result::Result::*; + match op { + #pyo3_path::basic::CompareOp::Eq => { + #eq_arm + } + #pyo3_path::basic::CompareOp::Ne => { + #ne_arm + } + #richcmp_ord + _ => { + Ok(py.NotImplemented()) + }, + } + } + }; + let richcmp_slot = + generate_default_protocol_slot(&ty, &mut richcmp_impl, &__RICHCMP__, ctx).unwrap(); + (richcmp_impl, richcmp_slot) +} + fn impl_simple_enum( simple_enum: PyClassSimpleEnum<'_>, args: &PyClassArgs, @@ -735,7 +837,6 @@ fn impl_simple_enum( methods_type: PyClassMethodsType, ctx: &Ctx, ) -> Result { - let Ctx { pyo3_path } = ctx; let cls = simple_enum.ident; let ty: syn::Type = syn::parse_quote!(#cls); let variants = simple_enum.variants; @@ -787,7 +888,12 @@ fn impl_simple_enum( (int_impl, int_slot) }; - let comparison_arm = |op| { + // Because PartialEq was not historically required on simple enums, we need to use the older + // logic to compute these arms; otherwise, compile time errors will arise requiring the user + // to implement the PartialEq trait. In a future PR, this trait will become a requirement + // for simple enums. + let Ctx { pyo3_path } = ctx; + let comparison_arm = |op: TokenStream| { quote! { let self_val = self.__pyo3__int__(); if let Ok(i) = other.extract::<#repr_type>() { @@ -800,65 +906,10 @@ fn impl_simple_enum( return Ok(py.NotImplemented()); } }; - let eq_arm = comparison_arm(quote! {==}); - let ne_arm = comparison_arm(quote! {!=}); + let (eq_arm, ne_arm) = (comparison_arm(quote! {==}), comparison_arm(quote! {!=})); - // Add ordering comparisons if ord option is passed to pyclass - let richcmp_ord = match args.options.ord { - Some(_) => { - let gt_arm = comparison_arm(quote! {>}); - let lt_arm = comparison_arm(quote! {<}); - let ge_arm = comparison_arm(quote! {>=}); - let le_arm = comparison_arm(quote! {<=}); - quote! { - #pyo3_path::basic::CompareOp::Gt => { - #gt_arm - } - #pyo3_path::basic::CompareOp::Lt => { - #lt_arm - } - #pyo3_path::basic::CompareOp::Le => { - #le_arm - } - #pyo3_path::basic::CompareOp::Ge => { - #ge_arm - } - } - } - _ => { - quote! {} - } - }; - - let (default_richcmp, default_richcmp_slot) = { - let mut richcmp_impl: syn::ImplItemFn = syn::parse_quote! { - fn __pyo3__richcmp__( - &self, - py: #pyo3_path::Python, - other: &#pyo3_path::Bound<'_, #pyo3_path::PyAny>, - op: #pyo3_path::basic::CompareOp - ) -> #pyo3_path::PyResult<#pyo3_path::PyObject> { - use #pyo3_path::conversion::ToPyObject; - use #pyo3_path::types::PyAnyMethods; - use ::core::result::Result::*; - match op { - #pyo3_path::basic::CompareOp::Eq => { - #eq_arm - } - #pyo3_path::basic::CompareOp::Ne => { - #ne_arm - } - #richcmp_ord - _ => { - Ok(py.NotImplemented()) - }, - } - } - }; - let richcmp_slot = - generate_default_protocol_slot(&ty, &mut richcmp_impl, &__RICHCMP__, ctx).unwrap(); - (richcmp_impl, richcmp_slot) - }; + let (default_richcmp, default_richcmp_slot) = + impl_cmp_ops(args, ctx, &ty, Some((eq_arm, ne_arm))); let default_slots = vec![default_repr_slot, default_int_slot, default_richcmp_slot]; @@ -900,13 +951,6 @@ fn impl_complex_enum( ) -> Result { let Ctx { pyo3_path } = ctx; - // flag invalid ord option - ensure_spanned!( - args.options.ord.is_none(), - args.options.ord.span() => - "The `ord` option of #[pyclass] is not supported for complex enums." - ); - // Need to rig the enum PyClass options let args = { let mut rigged_args = args.clone(); @@ -922,7 +966,20 @@ fn impl_complex_enum( let variants = complex_enum.variants; let pytypeinfo = impl_pytypeinfo(cls, &args, None, ctx); - let default_slots = vec![]; + // This match case is in place until an `eq` argument is added to the PyClassArgs options and + // will require PartialEq to be implemented. For now, equality is behind the `ord` argument + // for Python class complex enums, as PartialOrd requires PartialEq. + let (default_richcmp, default_slots) = match args.options.ord { + Some(_) => { + let ty = syn::parse_quote!(#cls); + let (default_richcmp, default_richcmp_slot) = impl_cmp_ops(&args, ctx, &ty, None); + ( + quote! {impl #cls {#default_richcmp}}, + vec![default_richcmp_slot], + ) + } + _ => (quote! {impl #cls {}}, vec![]), + }; let impl_builder = PyClassImplsBuilder::new( cls, @@ -1027,7 +1084,7 @@ fn impl_complex_enum( #[doc(hidden)] #[allow(non_snake_case)] - impl #cls {} + #default_richcmp #(#variant_cls_zsts)* From 708c9348450861b61a019b49e2df02f233e3b358 Mon Sep 17 00:00:00 2001 From: Michael Gilbert Date: Tue, 28 May 2024 07:16:34 -0700 Subject: [PATCH 13/24] update: adjusted compile time error checks for missing PartialOrd implementations. Refactored growing set of comparison tests for simple and complex enums and structs into separate test file. --- tests/test_class_comparisons.rs | 287 +++++++++++++++++++++++++++ tests/test_enum.rs | 52 ----- tests/ui/invalid_pyclass_args.stderr | 108 +++++++++- tests/ui/invalid_pyclass_enum.stderr | 108 +++++++++- 4 files changed, 497 insertions(+), 58 deletions(-) create mode 100644 tests/test_class_comparisons.rs diff --git a/tests/test_class_comparisons.rs b/tests/test_class_comparisons.rs new file mode 100644 index 00000000000..0fe0728ba44 --- /dev/null +++ b/tests/test_class_comparisons.rs @@ -0,0 +1,287 @@ +#![cfg(feature = "macros")] + +use pyo3::prelude::*; + +#[path = "../src/tests/common.rs"] +mod common; + +#[pyclass] +#[derive(Debug, Clone)] +pub enum MyEnum { + Variant, + OtherVariant, +} + +#[pyclass(ord)] +#[derive(Debug, PartialEq, Eq, Clone, PartialOrd)] +pub enum MyEnumOrd { + Variant, + OtherVariant, +} + +#[test] +fn test_enum_eq_enum() { + Python::with_gil(|py| { + let var1 = Py::new(py, MyEnum::Variant).unwrap(); + let var2 = Py::new(py, MyEnum::Variant).unwrap(); + let other_var = Py::new(py, MyEnum::OtherVariant).unwrap(); + py_assert!(py, var1 var2, "var1 == var2"); + py_assert!(py, var1 other_var, "var1 != other_var"); + py_assert!(py, var1 var2, "(var1 != var2) == False"); + }) +} + +#[test] +fn test_enum_eq_incomparable() { + Python::with_gil(|py| { + let var1 = Py::new(py, MyEnum::Variant).unwrap(); + py_assert!(py, var1, "(var1 == 'foo') == False"); + py_assert!(py, var1, "(var1 != 'foo') == True"); + }) +} + +#[test] +fn test_enum_ord_comparable_opt_in_only() { + Python::with_gil(|py| { + let var1 = Py::new(py, MyEnum::Variant).unwrap(); + let var2 = Py::new(py, MyEnum::OtherVariant).unwrap(); + // ordering on simple enums if opt in only, thus raising an error below + py_expect_exception!(py, var1 var2, "(var1 > var2) == False", PyTypeError); + }) +} + +#[test] +fn test_simple_enum_ord_comparable() { + Python::with_gil(|py| { + let var1 = Py::new(py, MyEnumOrd::Variant).unwrap(); + let var2 = Py::new(py, MyEnumOrd::OtherVariant).unwrap(); + let var3 = Py::new(py, MyEnumOrd::OtherVariant).unwrap(); + py_assert!(py, var1 var2, "(var1 > var2) == False"); + py_assert!(py, var1 var2, "(var1 < var2) == True"); + py_assert!(py, var1 var2, "(var1 >= var2) == False"); + py_assert!(py, var2 var3, "(var3 >= var2) == True"); + py_assert!(py, var1 var2, "(var1 <= var2) == True"); + }) +} + +#[pyclass(ord)] +#[derive(Debug, PartialEq, Eq, Clone, PartialOrd)] +pub enum MyComplexEnumOrd { + Variant(i32), + OtherVariant(String), +} + +#[pyclass(ord)] +#[derive(Debug, PartialEq, Eq, Clone, PartialOrd)] +pub enum MyComplexEnumOrd2 { + Variant { msg: String, idx: u32 }, + OtherVariant { name: String, idx: u32 }, +} + +#[test] +fn test_complex_enum_ord_comparable() { + Python::with_gil(|py| { + let var1 = Py::new(py, MyComplexEnumOrd::Variant(-2)).unwrap(); + let var2 = Py::new(py, MyComplexEnumOrd::Variant(5)).unwrap(); + let var3 = Py::new(py, MyComplexEnumOrd::OtherVariant("a".to_string())).unwrap(); + let var4 = Py::new(py, MyComplexEnumOrd::OtherVariant("b".to_string())).unwrap(); + py_assert!(py, var1 var2, "(var1 > var2) == False"); + py_assert!(py, var1 var2, "(var1 < var2) == True"); + py_assert!(py, var1 var2, "(var1 >= var2) == False"); + py_assert!(py, var1 var2, "(var1 <= var2) == True"); + + py_assert!(py, var1 var3, "(var1 >= var3) == False"); + py_assert!(py, var1 var3, "(var1 <= var3) == True"); + + py_assert!(py, var3 var4, "(var3 >= var4) == False"); + py_assert!(py, var3 var4, "(var3 <= var4) == True"); + + let var5 = Py::new( + py, + MyComplexEnumOrd2::Variant { + msg: "hello".to_string(), + idx: 1, + }, + ) + .unwrap(); + let var6 = Py::new( + py, + MyComplexEnumOrd2::Variant { + msg: "hello".to_string(), + idx: 1, + }, + ) + .unwrap(); + let var7 = Py::new( + py, + MyComplexEnumOrd2::Variant { + msg: "goodbye".to_string(), + idx: 7, + }, + ) + .unwrap(); + let var8 = Py::new( + py, + MyComplexEnumOrd2::Variant { + msg: "about".to_string(), + idx: 0, + }, + ) + .unwrap(); + let var9 = Py::new( + py, + MyComplexEnumOrd2::OtherVariant { + name: "albert".to_string(), + idx: 1, + }, + ) + .unwrap(); + + py_assert!(py, var5 var6, "(var5 == var6) == True"); + py_assert!(py, var5 var6, "(var5 <= var6) == True"); + py_assert!(py, var6 var7, "(var6 <= var7) == False"); + py_assert!(py, var6 var7, "(var6 >= var7) == True"); + py_assert!(py, var5 var8, "(var5 > var8) == True"); + py_assert!(py, var8 var9, "(var9 > var8) == True"); + }) +} + +#[pyclass(ord)] +#[derive(Debug, PartialEq, Eq, Clone, PartialOrd)] +pub struct Point { + x: i32, + y: i32, + z: i32, +} + +#[test] +fn test_struct_numeric_ord_comparable() { + Python::with_gil(|py| { + let var1 = Py::new(py, Point { x: 10, y: 2, z: 3 }).unwrap(); + let var2 = Py::new(py, Point { x: 2, y: 2, z: 3 }).unwrap(); + let var3 = Py::new(py, Point { x: 1, y: 22, z: 4 }).unwrap(); + let var4 = Py::new(py, Point { x: 1, y: 3, z: 4 }).unwrap(); + let var5 = Py::new(py, Point { x: 1, y: 3, z: 4 }).unwrap(); + py_assert!(py, var1 var2, "(var1 > var2) == True"); + py_assert!(py, var1 var2, "(var1 <= var2) == False"); + py_assert!(py, var2 var3, "(var3 < var2) == True"); + py_assert!(py, var3 var4, "(var3 > var4) == True"); + py_assert!(py, var4 var5, "(var4 == var5) == True"); + py_assert!(py, var3 var5, "(var3 != var5) == True"); + }) +} + +#[pyclass(ord)] +#[derive(Debug, PartialEq, Eq, Clone, PartialOrd)] +pub struct Person { + surname: String, + given_name: String, +} + +#[test] +fn test_struct_string_ord_comparable() { + Python::with_gil(|py| { + let var1 = Py::new( + py, + Person { + surname: "zzz".to_string(), + given_name: "bob".to_string(), + }, + ) + .unwrap(); + let var2 = Py::new( + py, + Person { + surname: "aaa".to_string(), + given_name: "sally".to_string(), + }, + ) + .unwrap(); + let var3 = Py::new( + py, + Person { + surname: "eee".to_string(), + given_name: "qqq".to_string(), + }, + ) + .unwrap(); + let var4 = Py::new( + py, + Person { + surname: "ddd".to_string(), + given_name: "aaa".to_string(), + }, + ) + .unwrap(); + + py_assert!(py, var1 var2, "(var1 > var2) == True"); + py_assert!(py, var1 var2, "(var1 <= var2) == False"); + py_assert!(py, var1 var3, "(var1 >= var3) == True"); + py_assert!(py, var2 var3, "(var2 >= var3) == False"); + py_assert!(py, var3 var4, "(var3 >= var4) == True"); + py_assert!(py, var3 var4, "(var3 != var4) == True"); + }) +} + +#[pyclass(ord)] +#[derive(Debug, PartialEq, Eq, Clone)] +pub struct Record { + name: String, + title: String, + idx: u32, +} + +impl PartialOrd for Record { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.idx.partial_cmp(&other.idx).unwrap()) + } +} + +#[test] +fn test_struct_custom_ord_comparable() { + Python::with_gil(|py| { + let var1 = Py::new( + py, + Record { + name: "zzz".to_string(), + title: "bbb".to_string(), + idx: 9, + }, + ) + .unwrap(); + let var2 = Py::new( + py, + Record { + name: "ddd".to_string(), + title: "aaa".to_string(), + idx: 1, + }, + ) + .unwrap(); + let var3 = Py::new( + py, + Record { + name: "vvv".to_string(), + title: "ggg".to_string(), + idx: 19, + }, + ) + .unwrap(); + let var4 = Py::new( + py, + Record { + name: "vvv".to_string(), + title: "ggg".to_string(), + idx: 19, + }, + ) + .unwrap(); + + py_assert!(py, var1 var2, "(var1 > var2) == True"); + py_assert!(py, var1 var2, "(var1 <= var2) == False"); + py_assert!(py, var1 var3, "(var1 >= var3) == False"); + py_assert!(py, var2 var3, "(var2 >= var3) == False"); + py_assert!(py, var3 var4, "(var3 == var4) == True"); + py_assert!(py, var2 var4, "(var2 != var4) == True"); + }) +} diff --git a/tests/test_enum.rs b/tests/test_enum.rs index e3f40cb0794..7c018708ca8 100644 --- a/tests/test_enum.rs +++ b/tests/test_enum.rs @@ -13,13 +13,6 @@ pub enum MyEnum { OtherVariant, } -#[pyclass(ord)] -#[derive(Debug, PartialEq, Eq, Clone)] -pub enum MyEnumOrd { - Variant, - OtherVariant, -} - #[test] fn test_enum_class_attr() { Python::with_gil(|py| { @@ -59,51 +52,6 @@ fn test_enum_arg() { }) } -#[test] -fn test_enum_eq_enum() { - Python::with_gil(|py| { - let var1 = Py::new(py, MyEnum::Variant).unwrap(); - let var2 = Py::new(py, MyEnum::Variant).unwrap(); - let other_var = Py::new(py, MyEnum::OtherVariant).unwrap(); - py_assert!(py, var1 var2, "var1 == var2"); - py_assert!(py, var1 other_var, "var1 != other_var"); - py_assert!(py, var1 var2, "(var1 != var2) == False"); - }) -} - -#[test] -fn test_enum_eq_incomparable() { - Python::with_gil(|py| { - let var1 = Py::new(py, MyEnum::Variant).unwrap(); - py_assert!(py, var1, "(var1 == 'foo') == False"); - py_assert!(py, var1, "(var1 != 'foo') == True"); - }) -} - -#[test] -fn test_enum_ord_comparable_opt_in_only() { - Python::with_gil(|py| { - let var1 = Py::new(py, MyEnum::Variant).unwrap(); - let var2 = Py::new(py, MyEnum::OtherVariant).unwrap(); - // ordering on simple enums if opt in only, thus raising an error below - py_expect_exception!(py, var1 var2, "(var1 > var2) == False", PyTypeError); - }) -} - -#[test] -fn test_enum_ord_comparable() { - Python::with_gil(|py| { - let var1 = Py::new(py, MyEnumOrd::Variant).unwrap(); - let var2 = Py::new(py, MyEnumOrd::OtherVariant).unwrap(); - let var3 = Py::new(py, MyEnumOrd::OtherVariant).unwrap(); - py_assert!(py, var1 var2, "(var1 > var2) == False"); - py_assert!(py, var1 var2, "(var1 < var2) == True"); - py_assert!(py, var1 var2, "(var1 >= var2) == False"); - py_assert!(py, var2 var3, "(var3 >= var2) == True"); - py_assert!(py, var1 var2, "(var1 <= var2) == True"); - }) -} - #[pyclass] enum CustomDiscriminant { One = 1, diff --git a/tests/ui/invalid_pyclass_args.stderr b/tests/ui/invalid_pyclass_args.stderr index b544a756ce9..3cd3bb6dbc1 100644 --- a/tests/ui/invalid_pyclass_args.stderr +++ b/tests/ui/invalid_pyclass_args.stderr @@ -58,8 +58,110 @@ error: a `#[pyclass]` cannot be both a `mapping` and a `sequence` 31 | struct CannotBeMappingAndSequence {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: The `ord` option of #[pyclass] is not supported for structs. - --> tests/ui/invalid_pyclass_args.rs:33:11 +error[E0369]: binary operation `==` cannot be applied to type `&InvalidOrderedStruct` + --> tests/ui/invalid_pyclass_args.rs:33:1 | 33 | #[pyclass(ord)] - | ^^^ + | ^^^^^^^^^^^^^^^ + | +note: an implementation of `PartialEq` might be missing for `InvalidOrderedStruct` + --> tests/ui/invalid_pyclass_args.rs:34:1 + | +34 | struct InvalidOrderedStruct { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ must implement `PartialEq` + = note: this error originates in the attribute macro `pyclass` (in Nightly builds, run with -Z macro-backtrace for more info) +help: consider annotating `InvalidOrderedStruct` with `#[derive(PartialEq)]` + | +34 + #[derive(PartialEq)] +35 | struct InvalidOrderedStruct { + | + +error[E0369]: binary operation `!=` cannot be applied to type `&InvalidOrderedStruct` + --> tests/ui/invalid_pyclass_args.rs:33:1 + | +33 | #[pyclass(ord)] + | ^^^^^^^^^^^^^^^ + | +note: an implementation of `PartialEq` might be missing for `InvalidOrderedStruct` + --> tests/ui/invalid_pyclass_args.rs:34:1 + | +34 | struct InvalidOrderedStruct { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ must implement `PartialEq` + = note: this error originates in the attribute macro `pyclass` (in Nightly builds, run with -Z macro-backtrace for more info) +help: consider annotating `InvalidOrderedStruct` with `#[derive(PartialEq)]` + | +34 + #[derive(PartialEq)] +35 | struct InvalidOrderedStruct { + | + +error[E0369]: binary operation `>` cannot be applied to type `&InvalidOrderedStruct` + --> tests/ui/invalid_pyclass_args.rs:33:1 + | +33 | #[pyclass(ord)] + | ^^^^^^^^^^^^^^^ + | +note: an implementation of `PartialOrd` might be missing for `InvalidOrderedStruct` + --> tests/ui/invalid_pyclass_args.rs:34:1 + | +34 | struct InvalidOrderedStruct { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ must implement `PartialOrd` + = note: this error originates in the attribute macro `pyclass` (in Nightly builds, run with -Z macro-backtrace for more info) +help: consider annotating `InvalidOrderedStruct` with `#[derive(PartialEq, PartialOrd)]` + | +34 + #[derive(PartialEq, PartialOrd)] +35 | struct InvalidOrderedStruct { + | + +error[E0369]: binary operation `<` cannot be applied to type `&InvalidOrderedStruct` + --> tests/ui/invalid_pyclass_args.rs:33:1 + | +33 | #[pyclass(ord)] + | ^^^^^^^^^^^^^^^ + | +note: an implementation of `PartialOrd` might be missing for `InvalidOrderedStruct` + --> tests/ui/invalid_pyclass_args.rs:34:1 + | +34 | struct InvalidOrderedStruct { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ must implement `PartialOrd` + = note: this error originates in the attribute macro `pyclass` (in Nightly builds, run with -Z macro-backtrace for more info) +help: consider annotating `InvalidOrderedStruct` with `#[derive(PartialEq, PartialOrd)]` + | +34 + #[derive(PartialEq, PartialOrd)] +35 | struct InvalidOrderedStruct { + | + +error[E0369]: binary operation `<=` cannot be applied to type `&InvalidOrderedStruct` + --> tests/ui/invalid_pyclass_args.rs:33:1 + | +33 | #[pyclass(ord)] + | ^^^^^^^^^^^^^^^ + | +note: an implementation of `PartialOrd` might be missing for `InvalidOrderedStruct` + --> tests/ui/invalid_pyclass_args.rs:34:1 + | +34 | struct InvalidOrderedStruct { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ must implement `PartialOrd` + = note: this error originates in the attribute macro `pyclass` (in Nightly builds, run with -Z macro-backtrace for more info) +help: consider annotating `InvalidOrderedStruct` with `#[derive(PartialEq, PartialOrd)]` + | +34 + #[derive(PartialEq, PartialOrd)] +35 | struct InvalidOrderedStruct { + | + +error[E0369]: binary operation `>=` cannot be applied to type `&InvalidOrderedStruct` + --> tests/ui/invalid_pyclass_args.rs:33:1 + | +33 | #[pyclass(ord)] + | ^^^^^^^^^^^^^^^ + | +note: an implementation of `PartialOrd` might be missing for `InvalidOrderedStruct` + --> tests/ui/invalid_pyclass_args.rs:34:1 + | +34 | struct InvalidOrderedStruct { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ must implement `PartialOrd` + = note: this error originates in the attribute macro `pyclass` (in Nightly builds, run with -Z macro-backtrace for more info) +help: consider annotating `InvalidOrderedStruct` with `#[derive(PartialEq, PartialOrd)]` + | +34 + #[derive(PartialEq, PartialOrd)] +35 | struct InvalidOrderedStruct { + | diff --git a/tests/ui/invalid_pyclass_enum.stderr b/tests/ui/invalid_pyclass_enum.stderr index df3c664a56d..2ca1ef9cca3 100644 --- a/tests/ui/invalid_pyclass_enum.stderr +++ b/tests/ui/invalid_pyclass_enum.stderr @@ -30,8 +30,110 @@ error: `constructor` can't be used on a simple enum variant 26 | #[pyo3(constructor = (a, b))] | ^^^^^^^^^^^ -error: The `ord` option of #[pyclass] is not supported for complex enums. - --> tests/ui/invalid_pyclass_enum.rs:31:11 +error[E0369]: binary operation `==` cannot be applied to type `&InvalidOrderedComplexEnum` + --> tests/ui/invalid_pyclass_enum.rs:31:1 | 31 | #[pyclass(ord)] - | ^^^ + | ^^^^^^^^^^^^^^^ + | +note: an implementation of `PartialEq` might be missing for `InvalidOrderedComplexEnum` + --> tests/ui/invalid_pyclass_enum.rs:32:1 + | +32 | enum InvalidOrderedComplexEnum { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ must implement `PartialEq` + = note: this error originates in the attribute macro `pyclass` (in Nightly builds, run with -Z macro-backtrace for more info) +help: consider annotating `InvalidOrderedComplexEnum` with `#[derive(PartialEq)]` + | +32 + #[derive(PartialEq)] +33 | enum InvalidOrderedComplexEnum { + | + +error[E0369]: binary operation `!=` cannot be applied to type `&InvalidOrderedComplexEnum` + --> tests/ui/invalid_pyclass_enum.rs:31:1 + | +31 | #[pyclass(ord)] + | ^^^^^^^^^^^^^^^ + | +note: an implementation of `PartialEq` might be missing for `InvalidOrderedComplexEnum` + --> tests/ui/invalid_pyclass_enum.rs:32:1 + | +32 | enum InvalidOrderedComplexEnum { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ must implement `PartialEq` + = note: this error originates in the attribute macro `pyclass` (in Nightly builds, run with -Z macro-backtrace for more info) +help: consider annotating `InvalidOrderedComplexEnum` with `#[derive(PartialEq)]` + | +32 + #[derive(PartialEq)] +33 | enum InvalidOrderedComplexEnum { + | + +error[E0369]: binary operation `>` cannot be applied to type `&InvalidOrderedComplexEnum` + --> tests/ui/invalid_pyclass_enum.rs:31:1 + | +31 | #[pyclass(ord)] + | ^^^^^^^^^^^^^^^ + | +note: an implementation of `PartialOrd` might be missing for `InvalidOrderedComplexEnum` + --> tests/ui/invalid_pyclass_enum.rs:32:1 + | +32 | enum InvalidOrderedComplexEnum { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ must implement `PartialOrd` + = note: this error originates in the attribute macro `pyclass` (in Nightly builds, run with -Z macro-backtrace for more info) +help: consider annotating `InvalidOrderedComplexEnum` with `#[derive(PartialEq, PartialOrd)]` + | +32 + #[derive(PartialEq, PartialOrd)] +33 | enum InvalidOrderedComplexEnum { + | + +error[E0369]: binary operation `<` cannot be applied to type `&InvalidOrderedComplexEnum` + --> tests/ui/invalid_pyclass_enum.rs:31:1 + | +31 | #[pyclass(ord)] + | ^^^^^^^^^^^^^^^ + | +note: an implementation of `PartialOrd` might be missing for `InvalidOrderedComplexEnum` + --> tests/ui/invalid_pyclass_enum.rs:32:1 + | +32 | enum InvalidOrderedComplexEnum { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ must implement `PartialOrd` + = note: this error originates in the attribute macro `pyclass` (in Nightly builds, run with -Z macro-backtrace for more info) +help: consider annotating `InvalidOrderedComplexEnum` with `#[derive(PartialEq, PartialOrd)]` + | +32 + #[derive(PartialEq, PartialOrd)] +33 | enum InvalidOrderedComplexEnum { + | + +error[E0369]: binary operation `<=` cannot be applied to type `&InvalidOrderedComplexEnum` + --> tests/ui/invalid_pyclass_enum.rs:31:1 + | +31 | #[pyclass(ord)] + | ^^^^^^^^^^^^^^^ + | +note: an implementation of `PartialOrd` might be missing for `InvalidOrderedComplexEnum` + --> tests/ui/invalid_pyclass_enum.rs:32:1 + | +32 | enum InvalidOrderedComplexEnum { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ must implement `PartialOrd` + = note: this error originates in the attribute macro `pyclass` (in Nightly builds, run with -Z macro-backtrace for more info) +help: consider annotating `InvalidOrderedComplexEnum` with `#[derive(PartialEq, PartialOrd)]` + | +32 + #[derive(PartialEq, PartialOrd)] +33 | enum InvalidOrderedComplexEnum { + | + +error[E0369]: binary operation `>=` cannot be applied to type `&InvalidOrderedComplexEnum` + --> tests/ui/invalid_pyclass_enum.rs:31:1 + | +31 | #[pyclass(ord)] + | ^^^^^^^^^^^^^^^ + | +note: an implementation of `PartialOrd` might be missing for `InvalidOrderedComplexEnum` + --> tests/ui/invalid_pyclass_enum.rs:32:1 + | +32 | enum InvalidOrderedComplexEnum { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ must implement `PartialOrd` + = note: this error originates in the attribute macro `pyclass` (in Nightly builds, run with -Z macro-backtrace for more info) +help: consider annotating `InvalidOrderedComplexEnum` with `#[derive(PartialEq, PartialOrd)]` + | +32 + #[derive(PartialEq, PartialOrd)] +33 | enum InvalidOrderedComplexEnum { + | From 38ab0d11ce59363c4e42df68160f7e1ba66e16e6 Mon Sep 17 00:00:00 2001 From: Michael Gilbert Date: Tue, 28 May 2024 07:45:02 -0700 Subject: [PATCH 14/24] fix: updated with clippy findings --- pyo3-macros-backend/src/pyclass.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs index eef13710b4a..4461df53f1b 100644 --- a/pyo3-macros-backend/src/pyclass.rs +++ b/pyo3-macros-backend/src/pyclass.rs @@ -361,7 +361,7 @@ fn impl_class( let (default_richcmp, default_slots) = match args.options.ord { Some(_) => { let ty = syn::parse_quote!(#cls); - let (default_richcmp, default_richcmp_slot) = impl_cmp_ops(&args, ctx, &ty, None); + let (default_richcmp, default_richcmp_slot) = impl_cmp_ops(args, ctx, &ty, None); ( quote! {impl #cls {#default_richcmp}}, vec![default_richcmp_slot], @@ -826,7 +826,7 @@ fn impl_cmp_ops( } }; let richcmp_slot = - generate_default_protocol_slot(&ty, &mut richcmp_impl, &__RICHCMP__, ctx).unwrap(); + generate_default_protocol_slot(ty, &mut richcmp_impl, &__RICHCMP__, ctx).unwrap(); (richcmp_impl, richcmp_slot) } From 455893d32526619629eadbe6530e03af5196fc44 Mon Sep 17 00:00:00 2001 From: Michael Gilbert Date: Tue, 28 May 2024 08:48:38 -0700 Subject: [PATCH 15/24] update: added not to pyclass parameters on ord (assumes that eq will be implemented and merged first) --- guide/pyclass-parameters.md | 1 + 1 file changed, 1 insertion(+) diff --git a/guide/pyclass-parameters.md b/guide/pyclass-parameters.md index 9bd0534ea5d..197c07ea8a4 100644 --- a/guide/pyclass-parameters.md +++ b/guide/pyclass-parameters.md @@ -12,6 +12,7 @@ | `mapping` | Inform PyO3 that this class is a [`Mapping`][params-mapping], and so leave its implementation of sequence C-API slots empty. | | `module = "module_name"` | Python code will see the class as being defined in this module. Defaults to `builtins`. | | `name = "python_name"` | Sets the name that Python sees this class as. Defaults to the name of the Rust struct. | +| `ord` | Implements ``__lt__`, `__gt__', `__le__`, & `__ge__` using the `PartialOrd` implementation of the underlying Rust datatype. *Requires that `eq` be passed as well.* | | `rename_all = "renaming_rule"` | Applies renaming rules to every getters and setters of a struct, or every variants of an enum. Possible values are: "camelCase", "kebab-case", "lowercase", "PascalCase", "SCREAMING-KEBAB-CASE", "SCREAMING_SNAKE_CASE", "snake_case", "UPPERCASE". | | `sequence` | Inform PyO3 that this class is a [`Sequence`][params-sequence], and so leave its C-API mapping length slot empty. | | `set_all` | Generates setters for all fields of the pyclass. | From 678806767f7110bbf2baac237b365d8997867f83 Mon Sep 17 00:00:00 2001 From: Michael Gilbert Date: Mon, 3 Jun 2024 16:33:22 -0700 Subject: [PATCH 16/24] update: rebased on main after merging of `eq` feature --- pyo3-macros-backend/src/pyclass.rs | 136 +++++++---------------------- tests/test_class_comparisons.rs | 16 ++-- 2 files changed, 40 insertions(+), 112 deletions(-) diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs index cf63e895335..4e348d36172 100644 --- a/pyo3-macros-backend/src/pyclass.rs +++ b/pyo3-macros-backend/src/pyclass.rs @@ -23,7 +23,7 @@ use syn::ext::IdentExt; use syn::parse::{Parse, ParseStream}; use syn::parse_quote_spanned; use syn::punctuated::Punctuated; -use syn::{parse_quote, spanned::Spanned, ImplItemFn, Result, Token}; +use syn::{parse_quote, spanned::Spanned, Result, Token}; /// If the class is derived from a Rust `struct` or `enum`. #[derive(Copy, Clone, Debug, PartialEq, Eq)] @@ -399,7 +399,6 @@ fn impl_class( Ok(quote! { impl #pyo3_path::types::DerefToPyAny for #cls {} - #default_richcmp #pytypeinfo_impl @@ -755,99 +754,6 @@ fn impl_enum( } } -/// Implements the rich comparison operations (`__eq__`, `__ne__`, `__gt__`, `__lt__`, `__ge__`, `__le__`) -/// for a PyClass. -/// -/// # Arguments -/// -/// * `args` - The PyClassArgs struct that contains information, such as arguments passed to the pyclass macro, about the PyClass. -/// * `ctx` - The Ctx struct that contains the pyo3_path. -/// * `ty` - The syn::Type of the PyClass. -/// * `eq_ne_arms` - An optional tuple containing the custom `__eq__` and `__ne__` arms. If not provided, -/// default comparison arms using the `==` and `!=` operators will be generated. -/// *Once `PartialEq` becomes required for simple enums, this argument will be removed.* -/// -/// # Returns -/// -/// A tuple containing: -/// * `richcmp_impl` - The generated rich comparison implementation as a syn::ImplItemFn. -/// * `richcmp_slot` - The generated protocol slot for `__richcmp__`. -fn impl_cmp_ops( - args: &PyClassArgs, - ctx: &Ctx, - ty: &syn::Type, - eq_ne_arms: Option<(TokenStream, TokenStream)>, -) -> (ImplItemFn, MethodAndSlotDef) { - let Ctx { pyo3_path } = ctx; - let comparison_arm = |op| { - quote! { - let other = &*other.downcast::()?.borrow(); - Ok((self #op other).to_object(py)) - } - }; - - let (eq_arm, ne_arm) = if let Some((eq_arm, ne_arm)) = eq_ne_arms { - (eq_arm, ne_arm) - } else { - (comparison_arm(quote! {==}), comparison_arm(quote! {!=})) - }; - - // Add ordering comparisons if ord option is passed to pyclass - let richcmp_ord = match args.options.ord { - Some(_) => { - let gt_arm = comparison_arm(quote! {>}); - let lt_arm = comparison_arm(quote! {<}); - let ge_arm = comparison_arm(quote! {>=}); - let le_arm = comparison_arm(quote! {<=}); - quote! { - #pyo3_path::basic::CompareOp::Gt => { - #gt_arm - } - #pyo3_path::basic::CompareOp::Lt => { - #lt_arm - } - #pyo3_path::basic::CompareOp::Le => { - #le_arm - } - #pyo3_path::basic::CompareOp::Ge => { - #ge_arm - } - } - } - _ => { - quote! {} - } - }; - - let mut richcmp_impl: syn::ImplItemFn = syn::parse_quote! { - fn __pyo3__richcmp__( - &self, - py: #pyo3_path::Python, - other: &#pyo3_path::Bound<'_, #pyo3_path::PyAny>, - op: #pyo3_path::basic::CompareOp - ) -> #pyo3_path::PyResult<#pyo3_path::PyObject> { - use #pyo3_path::conversion::ToPyObject; - use #pyo3_path::types::PyAnyMethods; - use ::core::result::Result::*; - match op { - #pyo3_path::basic::CompareOp::Eq => { - #eq_arm - } - #pyo3_path::basic::CompareOp::Ne => { - #ne_arm - } - #richcmp_ord - _ => { - Ok(py.NotImplemented()) - }, - } - } - }; - let richcmp_slot = - generate_default_protocol_slot(ty, &mut richcmp_impl, &__RICHCMP__, ctx).unwrap(); - (richcmp_impl, richcmp_slot) -} - fn impl_simple_enum( simple_enum: PyClassSimpleEnum<'_>, args: &PyClassArgs, @@ -1764,7 +1670,7 @@ fn impl_pytypeinfo( } } -fn pyclass_richcmp_arms(options: &PyClassPyO3Options, ctx: &Ctx) -> TokenStream { +fn pyclass_richcmp_arms(options: &PyClassPyO3Options, ctx: &Ctx) -> std::result::Result { let Ctx { pyo3_path } = ctx; let eq_arms = options @@ -1783,9 +1689,34 @@ fn pyclass_richcmp_arms(options: &PyClassPyO3Options, ctx: &Ctx) -> TokenStream }) .unwrap_or_default(); - // TODO: `ord` can be integrated here (#4202) - #[allow(clippy::let_and_return)] - eq_arms + if let Some(ord) = options.ord { + ensure_spanned!(options.eq.is_some(), ord.span() => "The `ord` option requires the `eq` option."); + } + + let ord_arms = options + .ord + .map(|ord| { + quote_spanned! { ord.span() => + #pyo3_path::pyclass::CompareOp::Gt => { + ::std::result::Result::Ok(#pyo3_path::conversion::IntoPy::into_py(self_val > other, py)) + }, + #pyo3_path::pyclass::CompareOp::Lt => { + ::std::result::Result::Ok(#pyo3_path::conversion::IntoPy::into_py(self_val < other, py)) + }, + #pyo3_path::pyclass::CompareOp::Le => { + ::std::result::Result::Ok(#pyo3_path::conversion::IntoPy::into_py(self_val <= other, py)) + }, + #pyo3_path::pyclass::CompareOp::Ge => { + ::std::result::Result::Ok(#pyo3_path::conversion::IntoPy::into_py(self_val >= other, py)) + }, + } + }) + .unwrap_or_else(|| {quote! {_ => ::std::result::Result::Ok(py.NotImplemented())}}); + + Ok(quote! { + #eq_arms + #ord_arms + }) } fn pyclass_richcmp_simple_enum( @@ -1822,7 +1753,7 @@ fn pyclass_richcmp_simple_enum( return Ok((None, None)); } - let arms = pyclass_richcmp_arms(&options, ctx); + let arms = pyclass_richcmp_arms(&options, ctx)?; let eq = options.eq.map(|eq| { quote_spanned! { eq.span() => @@ -1831,7 +1762,6 @@ fn pyclass_richcmp_simple_enum( let other = &*other.borrow(); return match op { #arms - _ => ::std::result::Result::Ok(py.NotImplemented()) } } } @@ -1845,7 +1775,6 @@ fn pyclass_richcmp_simple_enum( }) { return match op { #arms - _ => ::std::result::Result::Ok(py.NotImplemented()) } } } @@ -1885,7 +1814,7 @@ fn pyclass_richcmp( bail_spanned!(eq_int.span() => "`eq_int` can only be used on simple enums.") } - let arms = pyclass_richcmp_arms(options, ctx); + let arms = pyclass_richcmp_arms(options, ctx)?; if options.eq.is_some() { let mut richcmp_impl = parse_quote! { fn __pyo3__generated____richcmp__( @@ -1898,7 +1827,6 @@ fn pyclass_richcmp( let other = &*#pyo3_path::types::PyAnyMethods::downcast::(other)?.borrow(); match op { #arms - _ => ::std::result::Result::Ok(py.NotImplemented()) } } }; diff --git a/tests/test_class_comparisons.rs b/tests/test_class_comparisons.rs index 0fe0728ba44..3e1fc5be0ae 100644 --- a/tests/test_class_comparisons.rs +++ b/tests/test_class_comparisons.rs @@ -5,14 +5,14 @@ use pyo3::prelude::*; #[path = "../src/tests/common.rs"] mod common; -#[pyclass] -#[derive(Debug, Clone)] +#[pyclass(eq)] +#[derive(Debug, Clone, PartialEq)] pub enum MyEnum { Variant, OtherVariant, } -#[pyclass(ord)] +#[pyclass(eq,ord)] #[derive(Debug, PartialEq, Eq, Clone, PartialOrd)] pub enum MyEnumOrd { Variant, @@ -64,14 +64,14 @@ fn test_simple_enum_ord_comparable() { }) } -#[pyclass(ord)] +#[pyclass(eq,ord)] #[derive(Debug, PartialEq, Eq, Clone, PartialOrd)] pub enum MyComplexEnumOrd { Variant(i32), OtherVariant(String), } -#[pyclass(ord)] +#[pyclass(eq,ord)] #[derive(Debug, PartialEq, Eq, Clone, PartialOrd)] pub enum MyComplexEnumOrd2 { Variant { msg: String, idx: u32 }, @@ -146,7 +146,7 @@ fn test_complex_enum_ord_comparable() { }) } -#[pyclass(ord)] +#[pyclass(eq,ord)] #[derive(Debug, PartialEq, Eq, Clone, PartialOrd)] pub struct Point { x: i32, @@ -171,7 +171,7 @@ fn test_struct_numeric_ord_comparable() { }) } -#[pyclass(ord)] +#[pyclass(eq,ord)] #[derive(Debug, PartialEq, Eq, Clone, PartialOrd)] pub struct Person { surname: String, @@ -223,7 +223,7 @@ fn test_struct_string_ord_comparable() { }) } -#[pyclass(ord)] +#[pyclass(eq,ord)] #[derive(Debug, PartialEq, Eq, Clone)] pub struct Record { name: String, From 736a4b8facb18a446d7685f591e0fdcd93c3aac4 Mon Sep 17 00:00:00 2001 From: Michael Gilbert Date: Mon, 3 Jun 2024 16:33:47 -0700 Subject: [PATCH 17/24] update: format update --- pyo3-macros-backend/src/pyclass.rs | 7 +++++-- tests/test_class_comparisons.rs | 12 ++++++------ 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs index 4e348d36172..9cf46e0c794 100644 --- a/pyo3-macros-backend/src/pyclass.rs +++ b/pyo3-macros-backend/src/pyclass.rs @@ -1670,7 +1670,10 @@ fn impl_pytypeinfo( } } -fn pyclass_richcmp_arms(options: &PyClassPyO3Options, ctx: &Ctx) -> std::result::Result { +fn pyclass_richcmp_arms( + options: &PyClassPyO3Options, + ctx: &Ctx, +) -> std::result::Result { let Ctx { pyo3_path } = ctx; let eq_arms = options @@ -1692,7 +1695,7 @@ fn pyclass_richcmp_arms(options: &PyClassPyO3Options, ctx: &Ctx) -> std::result: if let Some(ord) = options.ord { ensure_spanned!(options.eq.is_some(), ord.span() => "The `ord` option requires the `eq` option."); } - + let ord_arms = options .ord .map(|ord| { diff --git a/tests/test_class_comparisons.rs b/tests/test_class_comparisons.rs index 3e1fc5be0ae..75bd6251aac 100644 --- a/tests/test_class_comparisons.rs +++ b/tests/test_class_comparisons.rs @@ -12,7 +12,7 @@ pub enum MyEnum { OtherVariant, } -#[pyclass(eq,ord)] +#[pyclass(eq, ord)] #[derive(Debug, PartialEq, Eq, Clone, PartialOrd)] pub enum MyEnumOrd { Variant, @@ -64,14 +64,14 @@ fn test_simple_enum_ord_comparable() { }) } -#[pyclass(eq,ord)] +#[pyclass(eq, ord)] #[derive(Debug, PartialEq, Eq, Clone, PartialOrd)] pub enum MyComplexEnumOrd { Variant(i32), OtherVariant(String), } -#[pyclass(eq,ord)] +#[pyclass(eq, ord)] #[derive(Debug, PartialEq, Eq, Clone, PartialOrd)] pub enum MyComplexEnumOrd2 { Variant { msg: String, idx: u32 }, @@ -146,7 +146,7 @@ fn test_complex_enum_ord_comparable() { }) } -#[pyclass(eq,ord)] +#[pyclass(eq, ord)] #[derive(Debug, PartialEq, Eq, Clone, PartialOrd)] pub struct Point { x: i32, @@ -171,7 +171,7 @@ fn test_struct_numeric_ord_comparable() { }) } -#[pyclass(eq,ord)] +#[pyclass(eq, ord)] #[derive(Debug, PartialEq, Eq, Clone, PartialOrd)] pub struct Person { surname: String, @@ -223,7 +223,7 @@ fn test_struct_string_ord_comparable() { }) } -#[pyclass(eq,ord)] +#[pyclass(eq, ord)] #[derive(Debug, PartialEq, Eq, Clone)] pub struct Record { name: String, From ff8184911ffed20c732f689e12a02f2b53356e1e Mon Sep 17 00:00:00 2001 From: Michael Gilbert Date: Mon, 3 Jun 2024 16:45:19 -0700 Subject: [PATCH 18/24] update: update all test output and doc tests --- guide/src/class.md | 2 +- tests/ui/invalid_pyclass_args.stderr | 10 +++- tests/ui/invalid_pyclass_enum.rs | 7 +++ tests/ui/invalid_pyclass_enum.stderr | 74 ++++++++++++++++++++++++++++ 4 files changed, 90 insertions(+), 3 deletions(-) diff --git a/guide/src/class.md b/guide/src/class.md index 10dcd2b204e..406a9430769 100644 --- a/guide/src/class.md +++ b/guide/src/class.md @@ -1165,7 +1165,7 @@ Ordering of enum variants is optionally added using `#[pyo3(ord)]`. ```rust # use pyo3::prelude::*; -#[pyclass(ord)] +#[pyclass(eq,ord)] #[derive(PartialEq, PartialOrd)] enum MyEnum{ A, diff --git a/tests/ui/invalid_pyclass_args.stderr b/tests/ui/invalid_pyclass_args.stderr index 8f1b671dfd9..23d3c3bbc64 100644 --- a/tests/ui/invalid_pyclass_args.stderr +++ b/tests/ui/invalid_pyclass_args.stderr @@ -1,4 +1,4 @@ -error: expected one of: `crate`, `dict`, `eq`, `eq_int`, `extends`, `freelist`, `frozen`, `get_all`, `hash`, `mapping`, `module`, `name`, `rename_all`, `sequence`, `set_all`, `subclass`, `unsendable`, `weakref` +error: expected one of: `crate`, `dict`, `eq`, `eq_int`, `extends`, `freelist`, `frozen`, `get_all`, `hash`, `mapping`, `module`, `name`, `ord`, `rename_all`, `sequence`, `set_all`, `subclass`, `unsendable`, `weakref` --> tests/ui/invalid_pyclass_args.rs:3:11 | 3 | #[pyclass(extend=pyo3::types::PyDict)] @@ -46,7 +46,7 @@ error: expected string literal 24 | #[pyclass(module = my_module)] | ^^^^^^^^^ -error: expected one of: `crate`, `dict`, `eq`, `eq_int`, `extends`, `freelist`, `frozen`, `get_all`, `hash`, `mapping`, `module`, `name`, `rename_all`, `sequence`, `set_all`, `subclass`, `unsendable`, `weakref` +error: expected one of: `crate`, `dict`, `eq`, `eq_int`, `extends`, `freelist`, `frozen`, `get_all`, `hash`, `mapping`, `module`, `name`, `ord`, `rename_all`, `sequence`, `set_all`, `subclass`, `unsendable`, `weakref` --> tests/ui/invalid_pyclass_args.rs:27:11 | 27 | #[pyclass(weakrev)] @@ -76,6 +76,12 @@ error: The `hash` option requires the `eq` option. 59 | #[pyclass(hash)] | ^^^^ +error: The `ord` option requires the `eq` option. + --> tests/ui/invalid_pyclass_args.rs:74:11 + | +74 | #[pyclass(ord)] + | ^^^ + error[E0592]: duplicate definitions with name `__pymethod___richcmp____` --> tests/ui/invalid_pyclass_args.rs:36:1 | diff --git a/tests/ui/invalid_pyclass_enum.rs b/tests/ui/invalid_pyclass_enum.rs index bbfb9ecd42f..c490f9291e3 100644 --- a/tests/ui/invalid_pyclass_enum.rs +++ b/tests/ui/invalid_pyclass_enum.rs @@ -86,4 +86,11 @@ enum InvalidOrderedComplexEnum { VariantB { msg: String } } +#[pyclass(eq,ord)] +#[derive(PartialEq)] +enum InvalidOrderedComplexEnum2 { + VariantA (i32), + VariantB { msg: String } +} + fn main() {} diff --git a/tests/ui/invalid_pyclass_enum.stderr b/tests/ui/invalid_pyclass_enum.stderr index cfa3922ef62..98ca2d77bfa 100644 --- a/tests/ui/invalid_pyclass_enum.stderr +++ b/tests/ui/invalid_pyclass_enum.stderr @@ -60,6 +60,12 @@ error: The `hash` option requires the `eq` option. 76 | #[pyclass(hash)] | ^^^^ +error: The `ord` option requires the `eq` option. + --> tests/ui/invalid_pyclass_enum.rs:83:11 + | +83 | #[pyclass(ord)] + | ^^^ + error[E0369]: binary operation `==` cannot be applied to type `&SimpleEqOptRequiresPartialEq` --> tests/ui/invalid_pyclass_enum.rs:31:11 | @@ -151,3 +157,71 @@ help: consider annotating `ComplexHashOptRequiresHash` with `#[derive(Hash)]` 64 + #[derive(Hash)] 65 | enum ComplexHashOptRequiresHash { | + +error[E0369]: binary operation `>` cannot be applied to type `&InvalidOrderedComplexEnum2` + --> tests/ui/invalid_pyclass_enum.rs:89:14 + | +89 | #[pyclass(eq,ord)] + | ^^^ + | +note: an implementation of `PartialOrd` might be missing for `InvalidOrderedComplexEnum2` + --> tests/ui/invalid_pyclass_enum.rs:91:1 + | +91 | enum InvalidOrderedComplexEnum2 { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ must implement `PartialOrd` +help: consider annotating `InvalidOrderedComplexEnum2` with `#[derive(PartialEq, PartialOrd)]` + | +91 + #[derive(PartialEq, PartialOrd)] +92 | enum InvalidOrderedComplexEnum2 { + | + +error[E0369]: binary operation `<` cannot be applied to type `&InvalidOrderedComplexEnum2` + --> tests/ui/invalid_pyclass_enum.rs:89:14 + | +89 | #[pyclass(eq,ord)] + | ^^^ + | +note: an implementation of `PartialOrd` might be missing for `InvalidOrderedComplexEnum2` + --> tests/ui/invalid_pyclass_enum.rs:91:1 + | +91 | enum InvalidOrderedComplexEnum2 { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ must implement `PartialOrd` +help: consider annotating `InvalidOrderedComplexEnum2` with `#[derive(PartialEq, PartialOrd)]` + | +91 + #[derive(PartialEq, PartialOrd)] +92 | enum InvalidOrderedComplexEnum2 { + | + +error[E0369]: binary operation `<=` cannot be applied to type `&InvalidOrderedComplexEnum2` + --> tests/ui/invalid_pyclass_enum.rs:89:14 + | +89 | #[pyclass(eq,ord)] + | ^^^ + | +note: an implementation of `PartialOrd` might be missing for `InvalidOrderedComplexEnum2` + --> tests/ui/invalid_pyclass_enum.rs:91:1 + | +91 | enum InvalidOrderedComplexEnum2 { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ must implement `PartialOrd` +help: consider annotating `InvalidOrderedComplexEnum2` with `#[derive(PartialEq, PartialOrd)]` + | +91 + #[derive(PartialEq, PartialOrd)] +92 | enum InvalidOrderedComplexEnum2 { + | + +error[E0369]: binary operation `>=` cannot be applied to type `&InvalidOrderedComplexEnum2` + --> tests/ui/invalid_pyclass_enum.rs:89:14 + | +89 | #[pyclass(eq,ord)] + | ^^^ + | +note: an implementation of `PartialOrd` might be missing for `InvalidOrderedComplexEnum2` + --> tests/ui/invalid_pyclass_enum.rs:91:1 + | +91 | enum InvalidOrderedComplexEnum2 { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ must implement `PartialOrd` +help: consider annotating `InvalidOrderedComplexEnum2` with `#[derive(PartialEq, PartialOrd)]` + | +91 + #[derive(PartialEq, PartialOrd)] +92 | enum InvalidOrderedComplexEnum2 { + | From 12599017ce20a0227e135c53b416603db895531b Mon Sep 17 00:00:00 2001 From: Michael Gilbert Date: Tue, 4 Jun 2024 10:42:04 -0700 Subject: [PATCH 19/24] Update guide/src/class.md Co-authored-by: Icxolu <10486322+Icxolu@users.noreply.github.com> --- guide/src/class.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/guide/src/class.md b/guide/src/class.md index 406a9430769..2bcfe75911e 100644 --- a/guide/src/class.md +++ b/guide/src/class.md @@ -1165,7 +1165,7 @@ Ordering of enum variants is optionally added using `#[pyo3(ord)]`. ```rust # use pyo3::prelude::*; -#[pyclass(eq,ord)] +#[pyclass(eq, ord)] #[derive(PartialEq, PartialOrd)] enum MyEnum{ A, From 14b57603b4e059b4ec24528721d4baab03bdae6d Mon Sep 17 00:00:00 2001 From: Michael Gilbert Date: Tue, 4 Jun 2024 10:42:32 -0700 Subject: [PATCH 20/24] Update pyo3-macros-backend/src/pyclass.rs Co-authored-by: Icxolu <10486322+Icxolu@users.noreply.github.com> --- pyo3-macros-backend/src/pyclass.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs index 9cf46e0c794..16d9beb50bd 100644 --- a/pyo3-macros-backend/src/pyclass.rs +++ b/pyo3-macros-backend/src/pyclass.rs @@ -1714,7 +1714,7 @@ fn pyclass_richcmp_arms( }, } }) - .unwrap_or_else(|| {quote! {_ => ::std::result::Result::Ok(py.NotImplemented())}}); + .unwrap_or_else(|| quote! { _ => ::std::result::Result::Ok(py.NotImplemented()) }); Ok(quote! { #eq_arms From b2851c7d5e816ca403f4c4f382520de225531692 Mon Sep 17 00:00:00 2001 From: Michael Gilbert Date: Tue, 4 Jun 2024 10:42:52 -0700 Subject: [PATCH 21/24] Update newsfragments/4202.added.md Co-authored-by: Icxolu <10486322+Icxolu@users.noreply.github.com> --- newsfragments/4202.added.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/newsfragments/4202.added.md b/newsfragments/4202.added.md index e1ebad5b223..d15b6ce810c 100644 --- a/newsfragments/4202.added.md +++ b/newsfragments/4202.added.md @@ -1 +1 @@ -Support optional `ord` argument to `#[pyclass]` for simple enums in the form of `#[pyclass(ord)]`. \ No newline at end of file +Add `#[pyclass(ord)]` to implement ordering based on `PartialOrd`. \ No newline at end of file From f8c782d0d99d8dceadd0238579c025803a943ee8 Mon Sep 17 00:00:00 2001 From: Michael Gilbert Date: Tue, 4 Jun 2024 10:44:04 -0700 Subject: [PATCH 22/24] Update guide/pyclass-parameters.md Co-authored-by: Icxolu <10486322+Icxolu@users.noreply.github.com> --- guide/pyclass-parameters.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/guide/pyclass-parameters.md b/guide/pyclass-parameters.md index 99dd9ee991a..470484ba20b 100644 --- a/guide/pyclass-parameters.md +++ b/guide/pyclass-parameters.md @@ -15,7 +15,7 @@ | `mapping` | Inform PyO3 that this class is a [`Mapping`][params-mapping], and so leave its implementation of sequence C-API slots empty. | | `module = "module_name"` | Python code will see the class as being defined in this module. Defaults to `builtins`. | | `name = "python_name"` | Sets the name that Python sees this class as. Defaults to the name of the Rust struct. | -| `ord` | Implements ``__lt__`, `__gt__', `__le__`, & `__ge__` using the `PartialOrd` implementation of the underlying Rust datatype. *Requires that `eq` be passed as well.* | +| `ord` | Implements ``__lt__`, `__gt__', `__le__`, & `__ge__` using the `PartialOrd` implementation of the underlying Rust datatype. *Requires `eq`* | | `rename_all = "renaming_rule"` | Applies renaming rules to every getters and setters of a struct, or every variants of an enum. Possible values are: "camelCase", "kebab-case", "lowercase", "PascalCase", "SCREAMING-KEBAB-CASE", "SCREAMING_SNAKE_CASE", "snake_case", "UPPERCASE". | | `sequence` | Inform PyO3 that this class is a [`Sequence`][params-sequence], and so leave its C-API mapping length slot empty. | | `set_all` | Generates setters for all fields of the pyclass. | From 102b074419e830e75642bc2838b2c149004d0156 Mon Sep 17 00:00:00 2001 From: Michael Gilbert Date: Tue, 4 Jun 2024 10:52:39 -0700 Subject: [PATCH 23/24] update: added note about `ord` implementation with example. --- guide/src/class/object.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/guide/src/class/object.md b/guide/src/class/object.md index 3b775c2b438..c0d25cd0597 100644 --- a/guide/src/class/object.md +++ b/guide/src/class/object.md @@ -249,6 +249,16 @@ To implement `__eq__` using the Rust [`PartialEq`] trait implementation, the `eq struct Number(i32); ``` +To implement `__lt__`, `__le__`, `__gt__`, & `__ge__` using the Rust `PartialOrd` trait implementation, the `ord` option can be used. *Note: Requires `eq`.* + +```rust +# use pyo3::prelude::*; +# +#[pyclass(eq, ord)] +#[derive(PartialEq, PartialOrd)] +struct Number(i32); +``` + ### Truthyness We'll consider `Number` to be `True` if it is nonzero: From 574266d0fb569208b128d58028dced54fd4540fc Mon Sep 17 00:00:00 2001 From: Icxolu <10486322+Icxolu@users.noreply.github.com> Date: Fri, 7 Jun 2024 20:48:11 +0200 Subject: [PATCH 24/24] fix doc formatting --- guide/pyclass-parameters.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/guide/pyclass-parameters.md b/guide/pyclass-parameters.md index 470484ba20b..a3a4e1f0c7d 100644 --- a/guide/pyclass-parameters.md +++ b/guide/pyclass-parameters.md @@ -15,7 +15,7 @@ | `mapping` | Inform PyO3 that this class is a [`Mapping`][params-mapping], and so leave its implementation of sequence C-API slots empty. | | `module = "module_name"` | Python code will see the class as being defined in this module. Defaults to `builtins`. | | `name = "python_name"` | Sets the name that Python sees this class as. Defaults to the name of the Rust struct. | -| `ord` | Implements ``__lt__`, `__gt__', `__le__`, & `__ge__` using the `PartialOrd` implementation of the underlying Rust datatype. *Requires `eq`* | +| `ord` | Implements `__lt__`, `__gt__`, `__le__`, & `__ge__` using the `PartialOrd` implementation of the underlying Rust datatype. *Requires `eq`* | | `rename_all = "renaming_rule"` | Applies renaming rules to every getters and setters of a struct, or every variants of an enum. Possible values are: "camelCase", "kebab-case", "lowercase", "PascalCase", "SCREAMING-KEBAB-CASE", "SCREAMING_SNAKE_CASE", "snake_case", "UPPERCASE". | | `sequence` | Inform PyO3 that this class is a [`Sequence`][params-sequence], and so leave its C-API mapping length slot empty. | | `set_all` | Generates setters for all fields of the pyclass. |