pyo3 icon indicating copy to clipboard operation
pyo3 copied to clipboard

Introspection: Adds basic input type annotations

Open Tpt opened this issue 8 months ago • 6 comments

Implementation notes:

  • The type annotation is a plain string. Indeed, it is not possible at my knowledge to properly concatenate strings from const code (hence without macros), making advanced structs not possible. Moreover, because it is not possible to use generic types in const code, we won't be able to generate list[int] types at the moment but only list. This limitation will be lifted if min_generic_const_args unstable feature lands at some point. However it should be possible to write a macro that validates the types and wrap them in some TypeAnnotation(&'static str) struct. Note that because our macros are aware of the Option<T> parameters annotations like T | None are properly generated. We might expand that to other common containers like Vec or HashMap.
  • The stub generator autogenerates import with very naive code. I am not sure we want to keep autogenerating them with I fear more complexity or request the user to write them by hand. If we auto generate them we must make it more robust (maybe in a follow up?)
  • The stubs generator does not support maximum line length yet, the test now reformats the stubs before doing string equality
  • I have only written types for a few built-in elements to not overload this MR
  • I have not added test coverage of types generated from #[pyclass]. We will get that for free after merging #5087
  • The default when no type is set is typing.Any. It might be cleaner to make INPUT_TYPE an Option<&'static str> and use None as the default value to not set an annotation instead.

Tpt avatar Apr 24 '25 11:04 Tpt

Thank you for the review.

I guess we will miss the 0.25 release for this (my bad).

There is still a bunch of follow ups to get something working properly so it's definitely not a big deal.

Tpt avatar May 13 '25 11:05 Tpt

Thanks, given all the comments I think this is good to merge

Amazing!

(and I guess I can rebase the 0.25 branch to include this after all 😂)

I would feel better to have it in 0.26 to get time to do more testing, add more types across the codebase, deprecate the input_type and output_type methods...

Tpt avatar May 13 '25 11:05 Tpt

@Tpt seems like there was a conflict introduced in the merge queue for this one?

abrisco avatar May 19 '25 13:05 abrisco

@abrisco Yes! Rebasing this MR encovered some edge cases I have not considered (Self in signatures and PyO3-only magic methods like __traverse__). I hope to find some time soon to fix them.

Tpt avatar May 19 '25 13:05 Tpt

Rebase done. CI semverchecks is failing because this MR introduces a new associated constant to traits

The relevant bit to review 42c0cc664e96777b9f332dfc2a8d5928af3f76cc

Tpt avatar May 19 '25 15:05 Tpt

@davidhewitt This MR would love a second review after the rebases. The relevant additional commit is 42c0cc664e96777b9f332dfc2a8d5928af3f76cc

Tpt avatar Jun 02 '25 15:06 Tpt

Thank you for the review!

Tpt avatar Jun 17 '25 12:06 Tpt