Explicit function to convert to/from method parameters

[NewbiZ]

I have a set of structs that have rust_decimal::Decimal as fields, as well as method parameters.

I would like to expose these structs to Python, but rust_decimal::Decimal is defined in an external crate, and IntoPy<PyObject> as well, so I have no clue as to how I could tell PyO3 how to convert to/from these structs.

[birkenfeld]

You would have to define a "newtype wrapper" struct like

struct Decimal(rust_decimal::Decimal);

and implement the pyo3 traits for that.

Another way is to contribute integration to pyo3, which would make sense if we can agree that rust_decimal is "the most common" Rust decimal crate...

[NewbiZ]

I thought about creating a wrapper type, but to be honest I cannot justify all the amount of boilerplate it would require in my project. I would have to implement/forward all the zillions of traits that rust_decimal::Decimal provides for each operator and conversion, as well as all its specific methods.

At this point I am more keen to fork pyo3 and add rust_decimal::Decimal support, this seems like much less work (will contribute the PR in the original repo as well of course).

I'm just thinking I cannot be the only one to have this issue, and there are already quite a number of forks that I found created just for this very purpose.

I wondered if it would not be more flexible for PyO3 to provide a way of explicitely providing a conversion function for a specific parameter/type to be used instead of IntoPy<PyObject>::into_py, in the same vein as what serde does for custom serialization/deserialization of a field.
Something that would look like:

fn into_py_custom_foreign_type(self, py: Python<'_>) -> PyObject {
    [...]
}

#[pyfunction(convert = [b => into_py_custom_foreign_type])]
fn foo(a: u32, b: CustomForeignType) {
    [...]
}

[davidhewitt]

It looks like there is both rust_decimal and bigdecimal with roughly equal usage (rust_decimal just ahead). rust_decimal looks more feature-rich and better maintained, so I think arguments could be made to either support just rust_decimal based on maintenance or support both based on usage (e.g. we support both anyhow and eyre for error reporting, and I am open to us considering supporting time to add alongside chrono).

That said, the Decimal type in Python is not exposed via the C API as far as I'm aware. So far we have generally avoided adding support in PyO3 for such things. That's not to say we can't do it, but a discussion would be needed to consider where the scope of such support should fall.

On your original question, note that you can also annotate b with #[pyo3(from_py_with = "into_py_custom_foreign_type")], which is very similar to your proposed "convert".

[NewbiZ]

On your original question, note that you can also annotate b with #[pyo3(from_py_with = "into_py_custom_foreign_type")], which is very similar to your proposed "convert".

Oh that seems great, I did not know this macro. Is there an equivalent for returned types?

[davidhewitt]

There isn't, because it's already straightforward to just wrap return values with either a wrapper type or a function call (as you prefer).

(The advantage for having the macro for the inputs is that we can then in the PyO3 framework give hints to the caller about which function input failed to convert.)

[cardoe]

I've made a rough swag at adding this into pyo3 proper at https://github.com/cardoe/pyo3/tree/decimal right now there's no docs and the tests aren't great. I'm hoping someone like @davidhewitt can tell me if this approach is okay and I'll work at getting this PR worthy.

[adamreichold]

(Also note that Decimal::from_str_exact does not seem to be present in rust-decimal 1.0, so a higher minimum version should probably be required.)

[cardoe]

Thanks for the suggestions. I'll incorporate them.

[cardoe]

Do you have any suggestions for the TODO items by any chance?

[adamreichold]

should this check that the underlying type is of decimal.Decimal ?

I would advise against that as FromPyObject is not a downcast but rather something like a "conversion constructor", i.e. it would be good if it works with an integer in addition a decimal.Decimal.

should this use obj.call_method0(intern!(py, "str"))?

I think our PyAny::str calling PyObject_Str should be equivalent to that but simpler and potentially faster as well.

how do we handle the error gracefully?

IIRC this is an open issue, meaning that ToPyObject will need to become fallible for other conversions as well but we have not yet worked out a sufficiently ergonomic way forward on this meaning that you need to stick with the panics for now.

is PyTypeError the right thing?

I think that does depend on the underlying error condition but in this case I would generally gravitate more towards PyValueError if you do not decide to check for the value having the decimal.Decimal type.

[davidhewitt]

I think it's easier to discuss the TODO items as a PR (your existing commit would already be fine to open a PR for discussion).

I'm a little uneasy about the performance characteristics of this, you may want to benchmark it for your use case. It could be worth also asking upstream CPython about the feasibility of adding a C API for this, which might help us reduce conversion cost? (Speculating wildly, I'm not sure how interoperable the C representation actually is.)

[mejrs]

I thought about creating a wrapper type, but to be honest I cannot justify all the amount of boilerplate it would require in my project.

I think this boilerplate is much less than you expect, so I'd still lean towards this over providing the conversions ourselves. IMO we should only do conversions if we can do it in a "good" way, i.e. if there's a C api we can call. decimal has no such C api.

[cardoe]

@NewbiZ you might want to mark this as answered. #3016 has landed which adds support natively to pyo3 for Decimal via rust_decimal.

[cardoe]

Now part of the 0.19.0 release.