New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix IntoPyCallbackOutput paper cuts #2326
Changes from 8 commits
bcb1762
139cae1
7af2326
924cb28
f348c17
acae555
63f67f1
44f3bc7
ab0bf9e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,3 +11,4 @@ coverage: | |
ignore: | ||
- tests/*.rs | ||
- src/test_hygiene/*.rs | ||
- src/impl_/ghost.rs |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
/// If it does nothing, was it ever really there? 👻 | ||
/// | ||
/// This is code that is just type checked to e.g. create better compile errors, | ||
/// but that never affects anything at runtime, | ||
use crate::{IntoPy, PyErr, PyObject}; | ||
|
||
#[allow(clippy::wrong_self_convention)] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this lint actually necessary? I wonder if it's a hangover from experimentation. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, that hits the which is required by `Blah: IntoResult<Result<Blah, PyErr>>`
which is required by `Blah: IntoPyResult<Result<Blah, PyErr>>`
`&Blah: IntoPy<Py<PyAny>>`
which is required by `&Blah: IntoResult<Result<&Blah, PyErr>>`
which is required by `&Blah: IntoPyResult<Result<&Blah, PyErr>>`
`&mut Blah: IntoPy<Py<PyAny>>`
which is required by `&mut Blah: IntoResult<Result<&mut Blah, PyErr>>`
which is required by `&mut Blah: IntoPyResult<Result<&mut Blah, PyErr>>` There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah I see! Do you think we should name the method differently then? E.g. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure 👍 |
||
pub trait IntoPyResult<T> { | ||
fn into_py_result(&mut self) {} | ||
} | ||
|
||
impl<T> IntoPyResult<T> for T where T: IntoPy<PyObject> {} | ||
|
||
impl<T, E> IntoPyResult<T> for Result<T, E> | ||
where | ||
T: IntoPy<PyObject>, | ||
E: Into<PyErr>, | ||
{ | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I prefer this approach of code that doesn't actually do anything 😉 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
struct Blah; | ||
|
||
#[pyo3::pyfunction] | ||
fn blah() -> Blah{ | ||
Blah | ||
} | ||
|
||
fn main(){} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
error[E0599]: the method `into_py_result` exists for struct `Blah`, but its trait bounds were not satisfied | ||
--> tests/ui/missing_intopy.rs:3:1 | ||
| | ||
1 | struct Blah; | ||
| ------------ | ||
| | | ||
| method `into_py_result` not found for this | ||
| doesn't satisfy `Blah: IntoPy<Py<PyAny>>` | ||
| doesn't satisfy `Blah: IntoPyResult<Blah>` | ||
2 | | ||
3 | #[pyo3::pyfunction] | ||
| ^^^^^^^^^^^^^^^^^^^ method cannot be called on `Blah` due to unsatisfied trait bounds | ||
| | ||
= note: the following trait bounds were not satisfied: | ||
`Blah: IntoPy<Py<PyAny>>` | ||
which is required by `Blah: IntoPyResult<Blah>` | ||
note: the following trait must be implemented | ||
--> src/conversion.rs | ||
| | ||
| / pub trait IntoPy<T>: Sized { | ||
| | /// Performs the conversion. | ||
| | fn into_py(self, py: Python<'_>) -> T; | ||
| | } | ||
| |_^ | ||
= note: this error originates in the attribute macro `pyo3::pyfunction` (in Nightly builds, run with -Z macro-backtrace for more info) | ||
|
||
error[E0277]: the trait bound `Blah: IntoPyCallbackOutput<_>` is not satisfied | ||
--> tests/ui/missing_intopy.rs:3:1 | ||
| | ||
3 | #[pyo3::pyfunction] | ||
| ^^^^^^^^^^^^^^^^^^^ the trait `IntoPyCallbackOutput<_>` is not implemented for `Blah` | ||
| | ||
note: required by a bound in `pyo3::callback::convert` | ||
--> src/callback.rs | ||
| | ||
| T: IntoPyCallbackOutput<U>, | ||
| ^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `pyo3::callback::convert` | ||
= note: this error originates in the attribute macro `pyo3::pyfunction` (in Nightly builds, run with -Z macro-backtrace for more info) | ||
Comment on lines
+27
to
+38
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's unfortunate this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer this approach over what I did before. This won't actually call
into_py_result
if an user accidentally writes it for their type.