Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions newsfragments/5273.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Introspection: Fixes introspection of `__richcmp__`, `__concat__`, `__repeat__`, `__inplace_concat__` and `__inplace_repeat__`
1 change: 1 addition & 0 deletions pyo3-macros-backend/src/method.rs
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,7 @@ impl CallingConvention {
}
}

#[derive(Clone)]
pub struct FnSpec<'a> {
pub tp: FnType,
// Rust function name
Expand Down
3 changes: 2 additions & 1 deletion pyo3-macros-backend/src/pyfunction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,13 +96,14 @@ pub struct PyFunctionWarningAttribute {
pub span: Span,
}

#[derive(PartialEq)]
#[derive(PartialEq, Clone)]
pub enum PyFunctionWarningCategory {
Path(Path),
UserWarning,
DeprecationWarning, // TODO: unused for now, intended for pyo3(deprecated) special-case
}

#[derive(Clone)]
pub struct PyFunctionWarning {
pub message: LitStr,
pub category: PyFunctionWarningCategory,
Expand Down
3 changes: 2 additions & 1 deletion pyo3-macros-backend/src/pyfunction/signature.rs
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ impl ConstructorAttribute {
}
}

#[derive(Default)]
#[derive(Default, Clone)]
pub struct PythonSignature {
pub positional_parameters: Vec<String>,
pub positional_only_parameters: usize,
Expand All @@ -286,6 +286,7 @@ impl PythonSignature {
}
}

#[derive(Clone)]
pub struct FunctionSignature<'a> {
pub arguments: Vec<FnArg<'a>>,
pub python_signature: PythonSignature,
Expand Down
47 changes: 30 additions & 17 deletions pyo3-macros-backend/src/pyimpl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,12 @@ use crate::{
};
use proc_macro2::TokenStream;
use quote::{format_ident, quote};
use syn::ImplItemFn;
#[cfg(feature = "experimental-inspect")]
use syn::Ident;
use syn::{
parse::{Parse, ParseStream},
spanned::Spanned,
Result,
ImplItemFn, Result,
};

/// The mechanism used to collect `#[pymethods]` into the type object
Expand Down Expand Up @@ -354,22 +355,34 @@ fn method_introspection_code(spec: &FnSpec<'_>, parent: &syn::Type, ctx: &Ctx) -
let Ctx { pyo3_path, .. } = ctx;

let name = spec.python_name.to_string();
if matches!(
name.as_str(),
"__richcmp__"
| "__concat__"
| "__repeat__"
| "__inplace_concat__"
| "__inplace_repeat__"
| "__getbuffer__"
| "__releasebuffer__"
| "__traverse__"
| "__clear__"
) {
// This is not a magic Python method, ignore for now
// TODO: properly implement
return quote! {};

// __richcmp__ special case
if name == "__richcmp__" {
// We expend into each individual method
return ["__eq__", "__ne__", "__lt__", "__le__", "__gt__", "__ge__"]
.into_iter()
.map(|method_name| {
let mut spec = (*spec).clone();
spec.python_name = Ident::new(method_name, spec.python_name.span());
// We remove the CompareOp arg, this is safe because the signature is always the same
// First the other value to compare with then the CompareOp
// We cant to keep the first argument type, hence this hack
spec.signature.arguments.pop();
spec.signature.python_signature.positional_parameters.pop();
method_introspection_code(&spec, parent, ctx)
})
.collect();
}
// We map or ignore some magic methods
// TODO: this might create a naming conflict
let name = match name.as_str() {
"__concat__" => "__add__".into(),
"__repeat__" => "__mul__".into(),
"__inplace_concat__" => "__iadd__".into(),
"__inplace_repeat__" => "__imul__".into(),
"__getbuffer__" | "__releasebuffer__" | "__traverse__" | "__clear__" => return quote! {},
_ => name,
};

// We introduce self/cls argument and setup decorators
let mut first_argument = None;
Expand Down
26 changes: 19 additions & 7 deletions pytests/src/comparisons.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use pyo3::basic::CompareOp;
use pyo3::prelude::*;

#[pyclass]
Expand Down Expand Up @@ -81,6 +82,21 @@ impl Ordered {
}
}

#[pyclass]
struct OrderedRichCmp(i64);

#[pymethods]
impl OrderedRichCmp {
#[new]
fn new(value: i64) -> Self {
Self(value)
}

fn __richcmp__(&self, other: &Self, op: CompareOp) -> bool {
op.matches(self.0.cmp(&other.0))
}
}

#[pyclass]
struct OrderedDefaultNe(i64);

Expand Down Expand Up @@ -113,11 +129,7 @@ impl OrderedDefaultNe {
}

#[pymodule(gil_used = false)]
pub fn comparisons(m: &Bound<'_, PyModule>) -> PyResult<()> {
m.add_class::<Eq>()?;
m.add_class::<EqDefaultNe>()?;
m.add_class::<EqDerived>()?;
m.add_class::<Ordered>()?;
m.add_class::<OrderedDefaultNe>()?;
Ok(())
pub mod comparisons {
#[pymodule_export]
use super::{Eq, EqDefaultNe, EqDerived, Ordered, OrderedDefaultNe, OrderedRichCmp};
}
5 changes: 3 additions & 2 deletions pytests/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ mod pyo3_pytests {
use super::*;

#[pymodule_export]
use {consts::consts, pyclasses::pyclasses, pyfunctions::pyfunctions};
use {
comparisons::comparisons, consts::consts, pyclasses::pyclasses, pyfunctions::pyfunctions,
};

// Inserting to sys.modules allows importing submodules nicely from Python
// e.g. import pyo3_pytests.buf_and_str as bas
Expand All @@ -32,7 +34,6 @@ mod pyo3_pytests {
m.add_wrapped(wrap_pymodule!(awaitable::awaitable))?;
#[cfg(not(Py_LIMITED_API))]
m.add_wrapped(wrap_pymodule!(buf_and_str::buf_and_str))?;
m.add_wrapped(wrap_pymodule!(comparisons::comparisons))?;
#[cfg(not(Py_LIMITED_API))]
m.add_wrapped(wrap_pymodule!(datetime::datetime))?;
m.add_wrapped(wrap_pymodule!(dict_iter::dict_iter))?;
Expand Down
37 changes: 37 additions & 0 deletions pytests/stubs/comparisons.pyi
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
class Eq:
def __eq__(self, /, other: Eq) -> bool: ...
def __ne__(self, /, other: Eq) -> bool: ...
def __new__(cls, /, value: int) -> None: ...

class EqDefaultNe:
def __eq__(self, /, other: EqDefaultNe) -> bool: ...
Comment on lines +6 to +7
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The __ne__ method gets "defaulted" on this type, is it just object.__ne__ (i.e. due to inheritance)?

Or is there actually still a unique EqDefaultNe.__ne__ method? We might want to special case here if so.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit tricky: PyO3 generates its own __ne__ default implementation that negates __eq__. This is exactly the same behavior as object.__ne__. Adding an explicit EqDefaultNe.__ne__ would be indeed nicer but Mypy already property type check != with the current generated stubs because of the object.__ne__ fake inheritence. Happy to fix it in a follow up alongside the possible concat vs add conflict

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Totally makes sense, we can refine in a follow-up 👍

def __new__(cls, /, value: int) -> None: ...

class EqDerived:
def __new__(cls, /, value: int) -> None: ...

class Ordered:
def __eq__(self, /, other: Ordered) -> bool: ...
def __ge__(self, /, other: Ordered) -> bool: ...
def __gt__(self, /, other: Ordered) -> bool: ...
def __le__(self, /, other: Ordered) -> bool: ...
def __lt__(self, /, other: Ordered) -> bool: ...
def __ne__(self, /, other: Ordered) -> bool: ...
def __new__(cls, /, value: int) -> None: ...

class OrderedDefaultNe:
def __eq__(self, /, other: OrderedDefaultNe) -> bool: ...
def __ge__(self, /, other: OrderedDefaultNe) -> bool: ...
def __gt__(self, /, other: OrderedDefaultNe) -> bool: ...
def __le__(self, /, other: OrderedDefaultNe) -> bool: ...
def __lt__(self, /, other: OrderedDefaultNe) -> bool: ...
def __new__(cls, /, value: int) -> None: ...

class OrderedRichCmp:
def __eq__(self, /, other: OrderedRichCmp) -> bool: ...
def __ge__(self, /, other: OrderedRichCmp) -> bool: ...
def __gt__(self, /, other: OrderedRichCmp) -> bool: ...
def __le__(self, /, other: OrderedRichCmp) -> bool: ...
def __lt__(self, /, other: OrderedRichCmp) -> bool: ...
def __ne__(self, /, other: OrderedRichCmp) -> bool: ...
def __new__(cls, /, value: int) -> None: ...
7 changes: 5 additions & 2 deletions pytests/tests/test_comparisons.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
EqDerived,
Ordered,
OrderedDefaultNe,
OrderedRichCmp,
)
from typing_extensions import Self

Expand Down Expand Up @@ -132,8 +133,10 @@ def __ge__(self, other: Self) -> bool:
return self.x >= other.x


@pytest.mark.parametrize("ty", (Ordered, PyOrdered), ids=("rust", "python"))
def test_ordered(ty: Type[Union[Ordered, PyOrdered]]):
@pytest.mark.parametrize(
"ty", (Ordered, OrderedRichCmp, PyOrdered), ids=("rust", "rust-richcmp", "python")
)
def test_ordered(ty: Type[Union[Ordered, OrderedRichCmp, PyOrdered]]):
a = ty(0)
b = ty(0)
c = ty(1)
Expand Down
6 changes: 6 additions & 0 deletions src/types/boolobject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,9 @@ impl<'py> IntoPyObject<'py> for bool {
type Output = Borrowed<'py, 'py, Self::Target>;
type Error = Infallible;

#[cfg(feature = "experimental-inspect")]
const OUTPUT_TYPE: &'static str = "bool";

#[inline]
fn into_pyobject(self, py: Python<'py>) -> Result<Self::Output, Self::Error> {
Ok(PyBool::new(py, self))
Expand All @@ -157,6 +160,9 @@ impl<'py> IntoPyObject<'py> for &bool {
type Output = Borrowed<'py, 'py, Self::Target>;
type Error = Infallible;

#[cfg(feature = "experimental-inspect")]
const OUTPUT_TYPE: &'static str = bool::OUTPUT_TYPE;

#[inline]
fn into_pyobject(self, py: Python<'py>) -> Result<Self::Output, Self::Error> {
(*self).into_pyobject(py)
Expand Down
Loading