Added From<Bound<'py, T>> impl for PyClassInitializer<T>.
#4214
Conversation
|
Please add a test -- they're not just to ensure that this implementation is correct (and I agree, it looks trivial), they're also to ensure that as we refactor and change our APIs in the future, we don't break this. |
No problem, done! |
|
Actually something failed, sorry I'll fix it. |
tests/test_class_new.rs
Outdated
| #[test] | ||
| fn test_new_returns_bound() { | ||
| Python::with_gil(|py| { | ||
| let obj = NewReturnsBound::new(py).unwrap(); |
There was a problem hiding this comment.
This just calls new, it doesn't actually invoke the mechanism you added.
Take a look at test_new_existing in this file for how you can test it.
There was a problem hiding this comment.
Hmm that's weird, it definitely hits the new From impl indirectly in the call sequence (the macro-generated __pymethod___new____ calls IntoPyCallbackOutput::convert on the Bound<T> which calls <Bound<T> as Into<_>>::into and then From::from), but I guess its not direct enough for the coverage runner to detect. Thanks for the tip, I am very new to CI :)
There was a problem hiding this comment.
Oh wait, the Rust-side call obviously doesn't actually invoke the __pymethod___new____ stuff, duh! I'll fix it now
|
Thanks |
|
No problem, thanks for the merge! |
This is a tiny change that allows
Bound<T>to be returned from a pyclass's#[new]method, as is already supported forPy<T>.This flexibility would be convenient in certain cases; for example, it is often desirable to have a constructor for use on the Rust-side that returns the type as a Python object (
Py<T>orBound<T>) instead ofTdirectly (e.g. when you want to be able todowncastintoT::BaseClass). And as long your desired arguments are the same as for your#[new]method, you can just reuse the same constructor for use from both Python and Rust. However, you are currently limited to getting aPy<T>from this, which usually has to be followed by.into_bound(py)if you want to do anything useful with it. But if you could have the#[new]method returnBound<T>instead, as implemented by this PR, then it would be more ergonomic to call from Rust (while making no difference on the Python side). I also think it just makes sense intuitively, sinceBound<T>is essentially justPy<T>with superpowers.In my opinion this change is too trivial to warrant adding tests, but let me know if you disagree and I can add some.