-
Notifications
You must be signed in to change notification settings - Fork 741
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support exporting Rust enums to Python (full ADTs) #417
Comments
I haven't checked, but doing the same thing as boost python might work |
well, C++ enums are a fair bit different from Rust enums. |
116: feat(module) `Module.exports` uses the new `ExportKind` int enum for the `kind` value r=Hywan a=Hywan This patch creates a new `ExportKind` int enum defined as: ```python class ExportKind(IntEnum): FUNCTION = 1 MEMORY = 2 GLOBAL = 3 TABLE = 4 ``` Since pyo3 doesn't have a mapping from Rust to Python enum (cf PyO3/pyo3#417), we had to find a workaround to create it. I believe the approch taken in this PR isn't too ugly. The enum is created with [the functional API](https://docs.python.org/3/library/enum.html#functional-api), such as: ```python IntEnum("ExportKind", "FUNCTION MEMORY GLOBAL TABLE") ``` Variants order and variant names are defined by implementations on `ExportKind` in Rust. Then, this new `ExportKind` enum is used in `Module.exports` as values of the `kind` pair. Co-authored-by: Ivan Enderlin <ivan.enderlin@hoa-project.net>
We should split this into two issues. One is supporting old school Enums and the other is supporting ADTs. I'd suggest that for now having a pleasant way to define a python Enum in rust would be what this issue should focus on and if there's any state in any of the rust enum states we say not currently supported. |
Agreed with the above |
I have some messy code that you can use as a reference: https://salsa.debian.org/Kazan-team/simple-soft-float/-/blob/e19291a0b8eb17e9b52c09b8e670bf0fe3244989/src/python_macros.rs |
I've made this the full ADT issue and created #834 for tracking the simple case. |
Cool - that works too :-)
…On Wed, 25 Mar 2020 at 08:44, David Hewitt ***@***.***> wrote:
I've made this the full ADT issue and created #834
<#834> for tracking the simple case.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#417 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAGEJCALUIVNHIZ25Q5TUYLRJG75XANCNFSM4HAWD3VA>
.
|
For full ADTs I would initially limit it to the one pyclass per variant |
This is now released in 0.12 if people want to try it out. |
In that case, we should close this issue no? |
To clarify - in 0.12 we added |
Well the intial feedback seems to be that it works well enough (one less big switch statement - yay), |
I would be very open to having I made such a comment on the FromPyObject PR, but we did agree that it feels easier to use Also the complication with the
it seems clear enough that Design opinions are very welcome from anyone with a use case for this. |
I quite like the idea of using a dict for the |
Python introduced a new pattern matching syntax in 3.10, and I think if we can make Rust enum support that, this will be as close to ADT as we can get. According to PEP634, class patterns in Python are actually just a series of
|
Thanks, I see you have opened #2002 to get us moving towards this! I think as well as
I think we definitely need to create a class for the I'm not sure (and it may depend on Python's pattern matching logic) whether we want to have a method for each variant on the Enum class, or we want each variant to be a subclass of the original enum. Thinking about it from a user perspective, I think we want an enum like this in Rust: enum Point {
TwoD { x: f32, y: f32 },
ThreeD { x: f32, y: f32, z: f32 }
} to use in Python as point = Point.TwoD(x=1.0, y=3.4)
match point:
case Point.TwoD(x, y):
print(f"Got 2D point ({x}, {y})")
case Point.ThreeD(x, y, z):
print(f"Got 3D point ({x}, {y}, {z})") |
an expanded example with what I'd expect: pub enum MyEnum {
A,
B(),
C {},
D(i8, i16),
E {a: u8, b: String},
} should be matchable like so: match value:
case MyEnum.A: # A is an instance of MyEnum, just like Python enums
pass
case MyEnum.B(): # B is a subclass of MyEnum
pass
case MyEnum.C(): # C is a subclass of MyEnum
pass
case MyEnum.D(x, y): # D is a subclass of MyEnum
pass
case MyEnum.E(a=x, b=y): # E is a subclass of MyEnum
pass |
When we implement each Subclassing the |
you can have custom metaclasses in the C API: |
Got it. I think I see how to do that, I'll play with it and see if I can get it to work |
Alright, I've successfully set the metaclass to a custom class. Now I'm trying to figure out how to get Is there a specific slot I should use for the method? A specific signature? Edit: fixed typo |
Then you end up writing code like this. Also, wouldn't that make this entire feature unavailable on limited api/PyPy? |
iirc there isn't a specific slot, just add a method |
another method for creating a class with a metaclass is just calling the metaclass as could be done in python code...this does make it harder to create methods though... this is suggested in the docs for |
Good news, I now have the For now I'm simply using PyObject.ob_type to set the metaclass. We can experiment with other ways after I have that |
yay!
afaict you should actually use |
(Sorry for the duplicate comment, accidentally commented from my wrong account) Just to make sure I understand correctly, here's essentially what I'm doing now (pseudocode). // First, I create the metaclass
let metaclass: *mut PyTypeObject = TypeCreator {
name: "MyEnumMeta",
base_type: &mut PyType_Type,
__new__: PyType_Type.tp_new.unwrap() as *mut c_void,
__instancecheck__: fancy_enum_instancecheck,
}
.create();
// Next, I create the base class for the enum
let enumclass: *mut PyTypeObject = TypeCreator {
name: "MyEnum",
basicsize: std::mem::size_of::<PyCell<MyEnum>>(),
tp_dealloc: tp_dealloc::<MyEnum>,
is_basetype: true,
..enum_variant_properties
}
.create();
// Set the metaclass as the ob_type for the new enumclass
(*(enumclass as *mut PyObject)).ob_type = metaclass;
// Finally, I create a new child class for each enum variant
let enum_variant_a: *mut PyTypeObject = TypeCreator {
name: "A",
basicsize: std::mem::size_of::<PyCell<MyEnum>>(),
tp_dealloc: tp_dealloc::<MyEnum>,
__new__: variant_a__new__,
}
.create();
// Set the metaclass as the ob_type for each enum variant
(*(enum_variant_a as *mut PyObject)).ob_type = metaclass; This is all working correctly on the python side. I've correctly mimicked the behavior of the example here. I've confirmed this works even after changing the variant type in an That said, it seems this method for setting the metaclass could be better: // Set the metaclass as the ob_type for the new enumclass
(*(enumclass as *mut PyObject)).ob_type = metaclass;
By this do you mean set ob_type on the // Set the metaclass as the ob_type for the new enumclass
#[cfg(all(PyPy, not(Py_3_9)))]
{
(*enumclass).ob_type = metaclass;
}
#[cfg(not(all(PyPy, not(Py_3_9))))]
{
(*enumclass).ob_base.ob_base.ob_type = metaclass;
} Is this correct? Should I be using PyType_FromMetaclass to construct the enumclass and variants altogether? |
Alright, design question! Say I have this enum: enum MyEnum {
A(u8, u16),
B {
name: String
}
} From the python side, you can access the named field like this: assert MyEnum.B(name="hello").name == "hello" How do we access the tuple fields? I'm assuming we'll provide an implementation of variant = MyEnum.A(1, 2)
assert variant[0] == 1
assert variant[1] == 2 Should we also make the class iterable so you can destructure it as a tuple? If so, should it only be iterable when there is a tuple variant? variant = MyEnum.A(1, 2)
a, b = variant
assert a == 1 and b == 2 In both cases, do we disallow custom user implementations of |
I think the downside of not using
I think this is the only sensible option.
If I understand, there's a class per-variant, so we could make it so only the tuple variants are iterable?
For the C-style enums we came up with a technique which creates default implementations of |
A few more design questions considering this enum: #[pyclass]
enum MyEnum {
A(#[pyo3(get, set, name="first")] u8, #[pyo3(get, set)] u16),
B {
#[pyo3(get, set)]
name: String,
#[pyo3(get, set)]
number: u8
}
}
|
Here's what I'm leaning towards for each of these options:
Although this would be convenient, I think we should keep this as explicit as possible and continue requiring
I'm leaning towards no here. If you want a named value you should probably refactor your enum variant to be named. I'm even considering disallowing accessing unnamed fields from the python side altogether (admittedly mostly because I'm lazy and implementing all of this is taking longer than I'd hoped).
I don't think this will be necessary, as I don't think generating python accessors by default is a good idea |
If we go down this route of making fields private, does that have implications for how Python ... though actually, thinking about it, Python |
You would only be able to match on those fields that have been exposed to Python, but would always be able to match on the variants themselves. I just realized there's another side of this coin as well - how do you construct the enum variant? For my motivating use case I would want to be able to construct any of the enum variants, by simply passing in all of the fields. After constructing the variant, the field accessors wouldn't be useful at all. With over 20 fields in my enum, generating all of those accessors is a lot of unnecessary codegen. My proposal would be this:
|
This sounds fine to me!
This has a lot of overlap with a possible |
Hi! I have a crate that’s mostly a class hierarchy and a parser creating it. I want to add a All works fine, except for a How can I manually wrap this single enum in a Python class today to avoid making the abstraction worse? Can I tell Pyo3 to create some of the boilerplate and write the rest manually? |
Given the enum is using structs inside, you can start by adding Then you can create I think you should be able to generate a mostly equivalent Python API like this without too much manual work. |
Probably not the best approach, but I have an example of wrapping a Rust enum using subclass here: https://github.com/messense/py-promql-parser/blob/0c723319aff9cea931062b29c8d0ef58e0851364/src/expr.rs. |
I see, so you wrap it, manually add constructors, and if some other Python class has fields containing the struct, you implement a getter instead of using #[derive(Debug, Clone, PartialEq)]
enum Shape { Ellipse(Ellipse), Lines(Lines), ... }
#[derive(Debug, Clone, PartialEq)]
#[pyclass(name="Shape")]
struct PyShape(Shape);
#[pymethods]
impl PyShape {
#[new] // generics aren’t actually possible for #[new] yet …
fn new(inner: impl Into<Shape>) -> Self {
PyShape(inner.into())
}
}
// Struct containing the enum:
#[derive(Debug, Clone, PartialEq)]
#[pyclass]
pub struct ShapeDraw {
#[pyo3(get)]
pub pen: Pen,
pub shape: Shape,
}
#[pymethods]
impl ShapeDraw {
#[getter]
fn get_shape(&self) -> PyShape {
PyShape(self.shape.clone())
}
} Thank you for the help, sorry for hijacking this thread. Maybe adding some docs for current limitations and patterns to get around them would be a good idea? |
Those examples using tuple structs don't actually demonstrate what these would map to in pydantic. |
Full ADT support is holding back rust libraries from being used from python. E.g. as mentioned here: pemistahl/lingua-rs#177 - would be really nice if this wasn't the case. |
This issue replaces the closed #131.
We need to provide a way to export an enum to Python.
The text was updated successfully, but these errors were encountered: