pyo3 icon indicating copy to clipboard operation
pyo3 copied to clipboard

Runtime type information for objects crossing the Rust–Python boundary

Open CLOVIS-AI opened this issue 2 years ago • 10 comments

Thank you for contributing to pyo3!

Please consider adding the following to your pull request:

  • [x] an entry in CHANGELOG.md
  • ~~docs to all new functions and / or detail in the guide~~ Documentation will be added when the whole inspect module will be done
  • [x] tests for all new or changed functions

First step towards https://github.com/PyO3/pyo3/issues/2454. The goal of this PR is to provide runtime information about Python types for any object that is transferred to/from Python.

Example usage:

println!("a: {}", <(bool, usize, Vec<HashMap<String, u32>>)>::type_input());
a: Tuple[bool, int, Sequence[Mapping[str, int]]]

This PR introduces two new methods:

  • FromPyObject::type_input: the Python type when the object appears as a parameter to a Rust function,
  • IntoPy::type_output: the Python type when the object appears as a return value of a Rust function.

Both methods are implemented for the most important Rust types (primitives like usize, collections like HashMap). The default implementation is to return Any (which is the recommended type when the exact type is missing, and avoids this PR being a breaking change).

In the future, this will allow the inspection module to generate code calling type_input and type_output to delay the Python type generation until runtime, which is required because macros are executed before the Rust compiler resolves the types.

Future work:

  • Implementing these methods for all other objects (PyDict, hashbrown tables,...); this is fairly easy and may be done on a case-by-case basis when more precise information is needed
  • Automatically generating the precise type information as part of #[pyclass]

CLOVIS-AI avatar Jul 01 '22 13:07 CLOVIS-AI

You'll need to rebase on main to make CI work (because of https://github.com/PyO3/pyo3/pull/2489 ) :)

mejrs avatar Jul 02 '22 15:07 mejrs

@mejrs it's rebased :)

CLOVIS-AI avatar Jul 04 '22 06:07 CLOVIS-AI

Fixed cargo fmt and cargo clippy

CLOVIS-AI avatar Jul 04 '22 11:07 CLOVIS-AI

👍 looks like you were thinking along the same lines for a first step as me with this PR! Nice work!

So I guess the only question here is whether type_input should be used for downcast errors as suggested in #2454? I think I'd like to confirm that use case works before we introduce new public APIs.

Similarly I wonder if users can already use these to include function signatures in docstrings? Might need some macro support. One to think about, can probably implement this in a separate PR if we want it.

Just for completeness, do other use cases for these beyond the introspection API come to mind? I'm sure users will always surprise us 😊

davidhewitt avatar Jul 07 '22 07:07 davidhewitt

Also probably obvious from my other comments, though just wanted to add I'd prefer we merge this after 0.17 is released so that we allow ourselves time to iterate on this further if needed after merge.

davidhewitt avatar Jul 07 '22 07:07 davidhewitt

Just for completeness, do other use cases for these beyond the introspection API come to mind? I'm sure users will always surprise us blush

Well, this PR itself is just the building block of "what is the actual Python type of this type as seen from Python". I expect that this can be used for hundreds of different things :sweat:. Indeed, debugging/error information seems like a good use case.

So I guess the only question here is whether type_input should be used for downcast errors as suggested in #2454? I think I'd like to confirm that use case works before we introduce new public APIs.

'should' is your decision, but it definitely sounds like it 'could'. I don't think it should be a part of this PR though.

I'd prefer we merge this after 0.17 is released so that we allow ourselves time to iterate on this further if needed after merge.

Do you have an estimate on when that will be? I'm a bit tight on schedule and there's a lot of things to do until .pyi generation is a thing :)

CLOVIS-AI avatar Jul 10 '22 09:07 CLOVIS-AI

Tracking preparation for 0.17 at https://github.com/PyO3/pyo3/issues/2493 - TBH at the moment I'm working on getting those pieces complete whenever I can. I'd like it to be at most a week or two, though it depends how much time I can manage to free up to make progress.

davidhewitt avatar Jul 12 '22 07:07 davidhewitt

Fixed rustdoc & conflicts, rebased on current main, better commit messages.

CLOVIS-AI avatar Aug 01 '22 09:08 CLOVIS-AI

I'm not sure I understand why test fails. What should I do to fix it?

CLOVIS-AI avatar Aug 01 '22 09:08 CLOVIS-AI

I'm not sure I understand why test fails. What should I do to fix it?

Run TRYBUILD=overwrite cargo test to update ui tests output, see https://github.com/dtolnay/trybuild#workflow

messense avatar Aug 01 '22 09:08 messense

@CLOVIS-AI sorry for the long delay with this caused by the 0.17 release. I'm now able to widen bandwidth again, I propose let's merge this and then we can continue iterating on the stub generation? I think this PR is good as-is, just needs a rebase :)

davidhewitt avatar Sep 06 '22 06:09 davidhewitt

Updated the expected test output & rebased.

CLOVIS-AI avatar Sep 06 '22 08:09 CLOVIS-AI

Thanks! With apologies will need one final rebase as #2587 has conflicted on the CHANGELOG. (Really need to find some time to work on #2337 soon.)

Also the coverage job is reporting that a lot of new implementations are not covered by tests, it would be much appreciated if we could add a few more. I find it's much better to add coverage at the same time as new APIs wherever possible rather than hope it gets covered later 😄

davidhewitt avatar Sep 06 '22 18:09 davidhewitt

The rebase is done. I do not have the enough time on my hands at the moment to increase the code coverage.

I'm not familiar with GitHub's UI for code coverage so I have some trouble reading it, but it seems to me like it is complaining about all the one-liner functions that simply return a TypeInfo instance? It's not possible for a bug to hide there (they are just Rust structure instanciations).

CLOVIS-AI avatar Sep 06 '22 19:09 CLOVIS-AI

Understood. Given that I'm the source of the delay that caused you to lose your availability, seems reasonable to me to proceed to merge in that case.

davidhewitt avatar Sep 06 '22 21:09 davidhewitt