PyO3 / pyo3

Rust bindings for the Python interpreter
https://pyo3.rs
Apache License 2.0
12.37k stars 763 forks source link

Comparing enums by identity #3059

Open ricohageman opened 1 year ago

ricohageman commented 1 year ago

In python the convention is to compare enums by identity (Enum.A is Enum.A and Enum.A is not Enum.B) instead of equality (Enum.A == Enum.A and Enum.A != Enum.B). However, a straight forward implementation of enums in rust exposed to python using pyo3 does not support this. The documentation also only mentions comparison by equality.

For the particular use-case I'm working with, it is used like the following:

#[pyclass]
pub enum PyEnum {
   A,
   B,
   C,
}

#[pyclass(get_all)]
pub struct PyEnumHoldingStruct {
   py_enum: PyEnum,
   ...
}

#[pymethods]
impl PyEnumHoldingStruct {
   #[new]
   pub fn new(py_enum: PyEnum, ...) -> Self {
      Self {
         py_enum,
         ...
      }
}

#[pymodule]
fn py_module(_py: Python, m: &PyModule) -> PyResult<()> {
    m.add_class::<PyEnum>()?;
    m.add_class::<PyEnumHoldingStruct>()?;
}
import py_module

py_a = py_module.PyEnumHoldingStruct(
   py_module.PyEnum.A,
   ...
)

assert py_a.py_enum is py_module.PyEnum.A
assert py_a.py_enum is not py_module.PyEnum.B
assert py_a.py_enum is not py_module.PyEnum.C

In https://github.com/PyO3/pyo3/issues/2384 there is a description on how to use GILOnceCell and #[classattr] to return pointers to the same instance of an enum such that one can compare the enums by identity. Ideally there would be a way to achieve this result without that amount of boilerplate code per enum variant. Would it be possible to automatically generate this means of some setting in #[pyclass]?

I would be able to implement this if there is some guidance and a clear idea on how to support it.

davidhewitt commented 1 year ago

Hello, I agree with you that this is important; I've been aware of this shortcoming in PyO3's enum implementation and while I've wanted to fix it for a while I haven't had any time available to invest in this particular issue.

Your offer to help implement is appreciated, I'd be happy to feedback on the design choices with you and help steer an implementation.

To point you in the right direction, there's a couple of inter-related pieces which would be worth reading and thinking about:

ricohageman commented 1 year ago

or maybe we should be generating a different IntoPy implementation for C-like enums which returns existing objects

I believe this is the way to go, otherwise it's required to use the class attributes everywhere where the enum is created. See the test I've added in https://github.com/PyO3/pyo3/pull/3061/commits/82353cc23ba4dd5c0c43c2c17b2aba9fd3f0ff8e. Now it's possible to use MyEnum::Variant and not the pyo3 generated class attributes and still be able to compare the enum by identity.