Add stub generation support for overloads.#5957
Add stub generation support for overloads.#5957tychedelia wants to merge 9 commits intoPyO3:mainfrom
Conversation
|
Thank you very much for working on this large topic. I have not reviewed the MR in details yet but I have a few questions. About the #[pyo3(overloads = [
(foo: "int") -> "int",
(foo: "float") -> "float"
])]
fn my_function() {
}this enforces that only a single attribute set It would be great to provide more careful validations like checking if overloads are consistent with the default signature (same default value...). But I am fine to have that in follow-ups to avoid a too large MR. On the emitted stubs, the python typing doc state: Thank you aggain! |
Okay, I think that makes sense. I wasn't totally sure about the whether to go for single attr or multiple, but looking at your example I think I prefer this as well.
Yeah, mypy actually just caught this, I've fixed it up. |
|
Okay I'm waffling back and forth on the syntax, but happy to support whatever the project wants I'm agnostic. |
Tpt
left a comment
There was a problem hiding this comment.
Okay I'm waffling back and forth on the syntax, but happy to support whatever the project wants I'm agnostic.
I think I prefer:
#[pyo3(overloads = [
(foo: "int") -> "int",
(foo: "float") -> "float"
])]but I do not feel strongly so glad to change my mind on this topic. This is also something we can tweak in follow ups.
| #[serde(default)] | ||
| doc: Option<String>, | ||
| #[serde(default)] | ||
| overloads: Vec<ChunkOverload>, |
There was a problem hiding this comment.
open design question: do we want to have overloads field in the introspection JSON with the olverloads or be closer to the final Python stubs and just have multiple Function chunks with the @overload decorator (and not chunk for the base function)?
I feel that it's underlying a deeper question on if the introspection data must be a more "semantic" AST or more "syntactic" one closer to the Python stubs.
Co-authored-by: Thomas Tanon <thomas@pellissier-tanon.fr>
I agree with @Tpt here. With #[pyfunction(
overloads = [
(x: "int") -> "int",
(x: "str") -> "str",
],
signature = (x)
)]
fn with_overloads<'py>(x: Any<'py>) -> Any<'py> {
x
}I also wonder about variants where the signature itself goes with the overloads, e.g. #[pyfunction(
signature = [
(x: "int") -> "int",
(x: "str") -> "str",
(x)
],
)]
fn with_overloads<'py>(x: Any<'py>) -> Any<'py> {
x
}but I don't think that feels quite as good. Also, I haven't checked the implementation here yet, but I think we should be able to enforce the overloads are compatible with the signature at compile time (e.g. positional arguments, argument names, kwarg names, required arguments etc), and emit helpful errors where they disagree. |
I agree. Also, we still need an authoritative source of truth for the default values and the full signature as exposed to the Python interpreter. The other option would be to merge all the overloads together but it looks less intuitive for me.
+1. Glad to do it in a follow up if @tychedelia prefers not to do it. |
Partially addresses #5137.
We need support for overloads in stub generation in our project. I mostly copied the argument parsing code and tried to extract that as shared helpers for the overload support. I'm not totally sure about the error path, this may be a bit brittle, but I added some baseline tests to ensure it works.