introduce into_py_with/into_py_with_ref for #[derive(IntoPyObject, IntoPyObjectRef)]

[Icxolu]

This adds #[pyo3(into_py_with = ..., into_py_with_ref = ...)] field options to the #[derive(IntoPyObject, IntoPyObjectRef)] derive macros as a complement to #[pyo3(from_py_with= ...)] of #[derive(FromPyObject)].

This allows simple customization of the generated implementation, which allows (for example) the usage of the derive macro in cases where one or more fields do not implement IntoPyObject themselves.

One notable difference in the current implementation compared to from_py_with is the type of the argument. from_py_with takes it's argument via a string (mostly for historical reasons I believe), into_py_with takes it directly as a path, which has the advantage that we get better IDE support (proper syntax highlighting and autocomplete). For consistency I would propose to switch over from_py_with to the path syntax in a followup.

[davidhewitt]

Thanks, I think we indeed want this (and it matches from_py_with) 👍

I wonder a little bit about the two separate options but I don't think there is likely to be an alternative and the names make sense to me. (See below comment.)

[davidhewitt]

There's an interesting UX puzzle here where we can't tell users if they passed the wrong attribute (e.g. into_py_with_ref to derive(IntoPyObject)) because the two derives are expanded separately and (I think?) are not aware of whether the other derive is present.

I wonder, is there anything we can do to improve that?

I can't see any way to just have a single option for both derives, which would be tidy but I think not possible from a type system perspective?

[Icxolu]

There's an interesting UX puzzle here where we can't tell users if they passed the wrong attribute (e.g. into_py_with_ref to derive(IntoPyObject)) because the two derives are expanded separately and (I think?) are not aware of whether the other derive is present.

Yeah, they're not aware of each other and I don't think they can. The only way I can think of to make them error out in a wrong combination is to use two different top-level attributes instead nesting them in pyo3, as in
#[into_py_with = ...] and #[into_py_with_ref = ...] instead of #[pyo3(into_py_with = ..., into_py_with_ref = ...)]. That way each macro would consume only the option intended for it, and a "cannot find attribute `...` in this scope" error will be emitted otherwise. This would be doable, but a bit different from a usage point than our other options. What do you think?

I can't see any way to just have a single option for both derives, which would be tidy but I think not possible from a type system perspective?

I would be really nice, if we only needed a single option, but I don't see a good way to support that either. The two derives have different Self types, so the only way to support them in one function would be to downgrade the "by value" case to the "by ref" case, but that seemed pretty annoying and unintuitive to me.

[davidhewitt]

One option I see is we could require the functions to take Cow inputs, at a small runtime cost I guess. Maybe the compiler would inline it? 🤔

I think another alternative is instead of passing a function the user could pass a namespace x which we expect to have x::into_py and x::ref_into_py functions on it (bikeshed). This namespace idea has prior art in #[serde(with = "module")].

I somewhat like the namespace idea; I guess it still requires the user to have to write two functions but it might make it possible to provide better error messages, and forces consistency because the one attribute means that if both derives are used, they both have to go through this pathway?

(If only one derive is used, the namespace would only need to have that corresponding function.)

What do you think of those?

Downside of the namespace idea would be that it becomes hard for into_py_with to be used for anything other than these derive macros. Because functions and modules share separate namespaces we could just have the by-value case use x as a function and the by-ref case use x as a module to look up x::by_ref. That way into_py_with in other contexts can accept a function, but the function-and-namespace combo is definitely complicated for users 😬

[Icxolu]

Interesting ideas, I think both of these could work. Personally I would prefer the Cow approach. It seems conceptually simpler and strikes a nice balance between ease of use and flexibility. Often it's probably possible to make use of Deref and create a simple conversion while still allowing to handle both cases if necessary. For maximum performance (and flexibility) it's still possible to implement IntoPyObject manually (or via a wrapper type).

For the namespaces: I kind of dislike to be forced to create a module and not being allowed to specify the function name. With serde I usually just use serialize_with and deserialize_with together instead of with. It's fine if the functions get reused a lot, but is pretty annoying if many different conversions are needed. (But that's pretty subjective I guess.)

Downside of the namespace idea would be that it becomes hard for into_py_with to be used for anything other than these derive macros.

I'm actually not sure anymore whether we need it anywhere else. Initially I thought about pyo3(get), but recommending to create a manual #[getter] seems way easier (and maybe even shorter) than using into_py_with there. But having the option to use it somewhere else would of course be nice to have.

Because functions and modules share separate namespaces we could just have the by-value case use x as a function and the by-ref case use x as a module to look up x::by_ref. That way into_py_with in other contexts can accept a function, but the function-and-namespace combo is definitely complicated for users 😬

That sounds really cursed 😨

[davidhewitt]

For consistency I would propose to switch over from_py_with to the path syntax in a followup.

Yes please (with a deprecation on the existing string syntax).

[davidhewitt]

One option I see is we could require the functions to take Cow inputs, at a small runtime cost I guess. Maybe the compiler would inline it? 🤔

I think another alternative is instead of passing a function the user could pass a namespace x which we expect to have x::into_py and x::ref_into_py functions on it (bikeshed). This namespace idea has prior art in #[serde(with = "module")].

I somewhat like the namespace idea; I guess it still requires the user to have to write two functions but it might make it possible to provide better error messages, and forces consistency because the one attribute means that if both derives are used, they both have to go through this pathway?

(If only one derive is used, the namespace would only need to have that corresponding function.)

What do you think of those?

Downside of the namespace idea would be that it becomes hard for into_py_with to be used for anything other than these derive macros. Because functions and modules share separate namespaces we could just have the by-value case use x as a function and the by-ref case use x as a module to look up x::by_ref. That way into_py_with in other contexts can accept a function, but the function-and-namespace combo is definitely complicated for users 😬