rework complex enum field conversion

[Icxolu]

This reworks the complex enum field conversion to use IntoPyObject either by reference if available, or together with Clone if not.

I think in the future we should probably rework complex enums to make them use of the same infrastructure we already have for #[pyo3(get)].

Closes #4216
Xref #4651

[davidhewitt]

Thanks! Agreed this could use a refactor, probably that's easier after we remove ToPyObject and other backwards-compatibility arms from the #[pyo3(get)] code.

[ChristopherRabotin]

Hello,

Thanks for this implementation. Just two questions:

1. why can't this enum be Clone? If one adds #[derive(Clone)], the compiler gets upset.
2. why can't this enum be PartialEq either?

I'm trying to mimic the behavior in Python and in Rust of an Expression builder, and as these can become complex, it makes sense in my use-case to have them be clonable and enable equality checks.

Thanks

---

Edit: My use case currently requires me to define the enum as follows. Would it be possible for PyO3 to automatically convert the Box<_> to Py<_> in these cases so I don't need the cfg check?

#[derive(Debug)]
#[cfg_attr(not(feature = "python"), Clone, PartialEq)]
pub enum VectorExpr {
    Fixed {
        x: f64,
        y: f64,
        z: f64,
    }, // Unitless vector, for arbitrary computations
    Radius(StateSpec),
    Velocity(StateSpec),
    OrbitalMomentum(StateSpec),
    EccentricityVector(StateSpec),
    #[cfg(not(feature = "python"))]
    CrossProduct {
        a: Box<Self>,
        b: Box<Self>,
    },
    #[cfg(feature = "python")]
    CrossProduct {
        a: Py<VectorExpr>,
        b: Py<VectorExpr>,
    },
}

[Icxolu]

It can't be Clone, because Py is not clone. Cloning the Py requires a Python reference count increase, which requires that we are attached to the runtime, which we model with the Python<'py> token. Clone cannot provide the interface to prove that, so we offer the functionality via Py::clone_ref which requires the Python token.

For similar reasons Py does not implement PartialEq. To do the expected comparison we need to know the value of what we want to compare. In general this also requires us to be attached to the interpreter, and PartialEq can not guarantee that. One exception to this would be frozen pyclasses, which are able to get &T via Py::get. We're pretty careful with what blankets we provide, but you could use this if implement PartialEq manually if your class is frozen (or can be made so)

As for Box, we would need IntoPyObject and FromPyObject for Box<T> to make that work. However currently our blanket impl<T> FromPyObject<'_, '_> for T where T: PyClass + Clone overlaps with that and thus prevents this. This blanket has actually been problematic in a few cases and #5419 proposes to remove it, which could open a possibility to support Boxed values.

[ChristopherRabotin]

Understood, thanks for the details. Do you have any recommendations on how I should approach this enum I'm working on?