Add new trait IntoPyObject #2316
Conversation
| @@ -112,7 +112,7 @@ fn test_buffer_referenced() { | |||
| let input = vec![b' ', b'2', b'3']; | |||
| let gil = Python::acquire_gil(); | |||
| let py = gil.python(); | |||
| let instance: PyObject = TestBufferClass { | |||
| let instance = TestBufferClass { | |||
There was a problem hiding this comment.
Users migrating might have to remove a number of : PyObject annotations like this.
| @@ -122,9 +122,9 @@ fn test_polymorphic_container_does_not_accept_other_types() { | |||
|
|
|||
| let setattr = |value: PyObject| p.as_ref(py).setattr("inner", value); | |||
|
|
|||
| assert!(setattr(1i32.into_py(py)).is_err()); | |||
| assert!(setattr(1i32.into_object(py)).is_err()); | |||
There was a problem hiding this comment.
Users migrating might have to replace into_py with into_object where they to want PyObject output. I'm not sure how annoying this might be.
Worst case I think we instruct users to replace all .into_py() with .into_object(), and then they can case-by-case change back to into_py() where the better typing is helpful.
| @@ -431,6 +534,13 @@ impl IntoPy<Py<PyTuple>> for () { | |||
| } | |||
| } | |||
|
|
|||
| impl IntoPyObject for () { | |||
There was a problem hiding this comment.
Note that IntoPy<PyObject> for () becomes None, and IntoPy<Py<PyTuple>> becomes ().
IMO this is a bit annoying, and ().into_py() should be an empty tuple
However without specialization, we need to do some fudging in the macros to fix the () case as a #[pyfunction] return value. I'm pretty sure I know how to do this.
There was a problem hiding this comment.
To me, it would seem preferable to fix the special case of the type of the return value of functions that do not return anything instead of ingraining this into the conversion traits.
There was a problem hiding this comment.
@adamreichold I think that we're saying the same thing? At the moment the conversion traits encode this special case, however really this should just be part of the #[pyfunction] / #[pymethods] macro. (With min_specialization it could probably be part of the IntoPyCallbackOutput trait in the future, however I won't hold my breath.)
There was a problem hiding this comment.
Yes, certainly. I just wanted to echo
IMO this is a bit annoying, and ().into_py() should be an empty tuple
which sounded like you did not fully convince yourself yet.
|
I like it a lot. I agree that the |
| fn into_py(self, py: Python<'_>) -> Py<Self::Target>; | ||
|
|
||
| /// Performs the conversion. | ||
| fn into_object(self, py: Python<'_>) -> PyObject { |
There was a problem hiding this comment.
| fn into_object(self, py: Python<'_>) -> PyObject { | |
| fn into_object(self, py: Python<'_>) -> Py<PyAny> { |
I think this communicates the difference between these methods better.
2882: inspect: gate behind `experimental-inspect` feature r=davidhewitt a=davidhewitt This is the last thing I want to do before preparing 0.18 release. The `pyo3::inspect` functionality looks useful as a first step towards #2454. However, we don't actually make use of this anywhere within PyO3 yet (we could probably use it for better error messages). I think we also have open questions about the traits which I'd like to resolve before committing to these additional APIs. (For example, this PR adds `IntoPy::type_output`, which seems potentially misplaced to me, the `type_output` function probably wants to be on a non-generic trait e.g. `ToPyObject` or maybe #2316.) As such, I propose putting these APIs behind an `experimental-inspect` feature gate for now, and invite users who find them useful to contribute a finished-off design. Co-authored-by: David Hewitt <1939362+davidhewitt@users.noreply.github.com>
|
Closing as this is very outdated and #4041 is an attempt to kickstart future discussion in this space. |
This is throwing out a concept that I've been pondering for a while to progress with the design of the to-python conversion traits.
It replaces
IntoPy<T>generic trait with a traitIntoPyObjectwhich has an associated type defined as follows:The names are intended to line up with the
ToPyObjecttrait which has theto_objectmethod. We could also haveTryIntoPyObjecttrait, potentially, with a similar design.The advantages I see of doing this:
into_py()and getPy<T>rather thanPy<PyAny>, helping avoid to need an immediate cast to work with the Python object.IntoPy<T>exist for a given type.#[derive(IntoPyObject)].