Skip to content

Commit

Permalink
[RFC] split PyErr::new() into multiple methods
Browse files Browse the repository at this point in the history
Similar to call(), introduces new0(), new1() and new_args(), and
the same on Exception types as new_err0(), new_err1() and
new_err_args().

After the deprecation cycle, new1() could be renamed back to new(),
since it is the one used in the vast majority of cases.

I kept `PyErrArguments`, but it's now only implemented for tuples,
not for all Python-convertible types.
  • Loading branch information
birkenfeld committed Aug 24, 2024
1 parent 18bca6e commit 92b2dc7
Show file tree
Hide file tree
Showing 62 changed files with 343 additions and 215 deletions.
2 changes: 1 addition & 1 deletion examples/getitem/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ impl ExampleContainer {

return Ok(delta);
} else {
return Err(PyTypeError::new_err("Unsupported type"));
return Err(PyTypeError::new_err1("Unsupported type"));
}
}
fn __setitem__(&self, idx: IntOrSlice, value: u32) -> PyResult<()> {
Expand Down
2 changes: 1 addition & 1 deletion guide/src/class.md
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ impl Nonzero {
#[new]
fn py_new(value: i32) -> PyResult<Self> {
if value == 0 {
Err(PyValueError::new_err("cannot be zero"))
Err(PyValueError::new_err1("cannot be zero"))
} else {
Ok(Nonzero(value))
}
Expand Down
16 changes: 8 additions & 8 deletions guide/src/class/numeric.md
Original file line number Diff line number Diff line change
Expand Up @@ -95,28 +95,28 @@ impl Number {
fn __truediv__(&self, other: &Self) -> PyResult<Self> {
match self.0.checked_div(other.0) {
Some(i) => Ok(Self(i)),
None => Err(PyZeroDivisionError::new_err("division by zero")),
None => Err(PyZeroDivisionError::new_err1("division by zero")),
}
}

fn __floordiv__(&self, other: &Self) -> PyResult<Self> {
match self.0.checked_div(other.0) {
Some(i) => Ok(Self(i)),
None => Err(PyZeroDivisionError::new_err("division by zero")),
None => Err(PyZeroDivisionError::new_err1("division by zero")),
}
}

fn __rshift__(&self, other: &Self) -> PyResult<Self> {
match other.0.try_into() {
Ok(rhs) => Ok(Self(self.0.wrapping_shr(rhs))),
Err(_) => Err(PyValueError::new_err("negative shift count")),
Err(_) => Err(PyValueError::new_err1("negative shift count")),
}
}

fn __lshift__(&self, other: &Self) -> PyResult<Self> {
match other.0.try_into() {
Ok(rhs) => Ok(Self(self.0.wrapping_shl(rhs))),
Err(_) => Err(PyValueError::new_err("negative shift count")),
Err(_) => Err(PyValueError::new_err1("negative shift count")),
}
}
}
Expand Down Expand Up @@ -275,28 +275,28 @@ impl Number {
fn __truediv__(&self, other: &Self) -> PyResult<Self> {
match self.0.checked_div(other.0) {
Some(i) => Ok(Self(i)),
None => Err(PyZeroDivisionError::new_err("division by zero")),
None => Err(PyZeroDivisionError::new_err1("division by zero")),
}
}

fn __floordiv__(&self, other: &Self) -> PyResult<Self> {
match self.0.checked_div(other.0) {
Some(i) => Ok(Self(i)),
None => Err(PyZeroDivisionError::new_err("division by zero")),
None => Err(PyZeroDivisionError::new_err1("division by zero")),
}
}

fn __rshift__(&self, other: &Self) -> PyResult<Self> {
match other.0.try_into() {
Ok(rhs) => Ok(Self(self.0.wrapping_shr(rhs))),
Err(_) => Err(PyValueError::new_err("negative shift count")),
Err(_) => Err(PyValueError::new_err1("negative shift count")),
}
}

fn __lshift__(&self, other: &Self) -> PyResult<Self> {
match other.0.try_into() {
Ok(rhs) => Ok(Self(self.0.wrapping_shl(rhs))),
Err(_) => Err(PyValueError::new_err("negative shift count")),
Err(_) => Err(PyValueError::new_err1("negative shift count")),
}
}

Expand Down
6 changes: 3 additions & 3 deletions guide/src/exception.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ use pyo3::{Python, PyErr};
use pyo3::exceptions::PyTypeError;

Python::with_gil(|py| {
PyTypeError::new_err("Error").restore(py);
PyTypeError::new_err1("Error").restore(py);
assert!(PyErr::occurred(py));
drop(PyErr::fetch(py));
});
Expand Down Expand Up @@ -92,7 +92,7 @@ To check the type of an exception, you can similarly do:
# use pyo3::exceptions::PyTypeError;
# use pyo3::prelude::*;
# Python::with_gil(|py| {
# let err = PyTypeError::new_err(());
# let err = PyTypeError::new_err0();
err.is_instance_of::<PyTypeError>(py);
# });
```
Expand All @@ -113,7 +113,7 @@ mod io {

fn tell(file: &Bound<'_, PyAny>) -> PyResult<u64> {
match file.call_method0("tell") {
Err(_) => Err(io::UnsupportedOperation::new_err("not supported: tell")),
Err(_) => Err(io::UnsupportedOperation::new_err1("not supported: tell")),
Ok(x) => x.extract::<u64>(),
}
}
Expand Down
6 changes: 3 additions & 3 deletions guide/src/function/error-handling.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ use pyo3::prelude::*;
#[pyfunction]
fn check_positive(x: i32) -> PyResult<()> {
if x < 0 {
Err(PyValueError::new_err("x is negative"))
Err(PyValueError::new_err1("x is negative"))
} else {
Ok(())
}
Expand Down Expand Up @@ -109,7 +109,7 @@ impl fmt::Display for CustomIOError {

impl std::convert::From<CustomIOError> for PyErr {
fn from(err: CustomIOError) -> PyErr {
PyOSError::new_err(err.to_string())
PyOSError::new_err1(err.to_string())
}
}

Expand Down Expand Up @@ -205,7 +205,7 @@ struct MyOtherError(OtherError);

impl From<MyOtherError> for PyErr {
fn from(error: MyOtherError) -> Self {
PyValueError::new_err(error.0.message())
PyValueError::new_err1(error.0.message())
}
}

Expand Down
8 changes: 4 additions & 4 deletions guide/src/migration.md
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,7 @@ impl PyClassIter {
self.count += 1;
Ok(self.count)
} else {
Err(PyStopIteration::new_err("done"))
Err(PyStopIteration::new_err1("done"))
}
}
}
Expand Down Expand Up @@ -852,7 +852,7 @@ When converting from `anyhow::Error` or `eyre::Report` to `PyErr`, if the inner
# use pyo3::exceptions::PyValueError;
#[pyfunction]
fn raise_err() -> anyhow::Result<()> {
Err(PyValueError::new_err("original error message").into())
Err(PyValueError::new_err1("original error message").into())
}
fn main() {
Expand Down Expand Up @@ -1517,7 +1517,7 @@ let result: PyResult<()> = PyErr::new::<TypeError, _>("error message").into();
After (also using the new reworked exception types; see the following section):
```rust
# use pyo3::{PyResult, exceptions::PyTypeError};
let result: PyResult<()> = Err(PyTypeError::new_err("error message"));
let result: PyResult<()> = Err(PyTypeError::new_err1("error message"));
```
</details>

Expand All @@ -1541,7 +1541,7 @@ After:
# use pyo3::{PyErr, PyResult, Python, type_object::PyTypeObject};
# use pyo3::exceptions::{PyBaseException, PyTypeError};
# Python::with_gil(|py| -> PyResult<()> {
let err: PyErr = PyTypeError::new_err("error message");
let err: PyErr = PyTypeError::new_err1("error message");
// Uses Display for PyErr, new for PyO3 0.12
assert_eq!(err.to_string(), "TypeError: error message");
Expand Down
4 changes: 2 additions & 2 deletions guide/src/performance.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ fn frobnicate<'py>(value: &Bound<'py, PyAny>) -> PyResult<Bound<'py, PyAny>> {
} else if let Ok(vec) = value.extract::<Vec<Bound<'_, PyAny>>>() {
frobnicate_vec(vec)
} else {
Err(PyTypeError::new_err("Cannot frobnicate that type."))
Err(PyTypeError::new_err1("Cannot frobnicate that type."))
}
}
```
Expand All @@ -48,7 +48,7 @@ fn frobnicate<'py>(value: &Bound<'py, PyAny>) -> PyResult<Bound<'py, PyAny>> {
} else if let Ok(vec) = value.extract::<Vec<Bound<'_, PyAny>>>() {
frobnicate_vec(vec)
} else {
Err(PyTypeError::new_err("Cannot frobnicate that type."))
Err(PyTypeError::new_err1("Cannot frobnicate that type."))
}
}
```
Expand Down
4 changes: 2 additions & 2 deletions pyo3-benches/benches/bench_err.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@ use pyo3::{exceptions::PyValueError, prelude::*};
fn err_new_restore_and_fetch(b: &mut Bencher<'_>) {
Python::with_gil(|py| {
b.iter(|| {
PyValueError::new_err("some exception message").restore(py);
PyValueError::new_err1("some exception message").restore(py);
PyErr::fetch(py)
})
})
}

fn err_new_without_gil(b: &mut Bencher<'_>) {
b.iter(|| PyValueError::new_err("some exception message"))
b.iter(|| PyValueError::new_err1("some exception message"))
}

fn criterion_benchmark(c: &mut Criterion) {
Expand Down
2 changes: 1 addition & 1 deletion pyo3-macros-backend/src/pyclass.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1351,7 +1351,7 @@ fn impl_complex_enum_tuple_variant_getitem(
let py = slf.py();
match idx {
#( #match_arms, )*
_ => ::std::result::Result::Err(#pyo3_path::exceptions::PyIndexError::new_err("tuple index out of range")),
_ => ::std::result::Result::Err(#pyo3_path::exceptions::PyIndexError::new_err1("tuple index out of range")),
}
}
};
Expand Down
6 changes: 3 additions & 3 deletions pyo3-macros-backend/src/pymethod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -679,7 +679,7 @@ pub fn impl_py_setter_def(
use ::std::convert::Into;
let _value = #pyo3_path::impl_::pymethods::BoundRef::ref_from_ptr_or_opt(py, &_value)
.ok_or_else(|| {
#pyo3_path::exceptions::PyAttributeError::new_err("can't delete attribute")
#pyo3_path::exceptions::PyAttributeError::new_err1("can't delete attribute")
})?;
#init_holders
#extract
Expand Down Expand Up @@ -1100,15 +1100,15 @@ impl Ty {
Ty::CompareOp => extract_error_mode.handle_error(
quote! {
#pyo3_path::class::basic::CompareOp::from_raw(#ident)
.ok_or_else(|| #pyo3_path::exceptions::PyValueError::new_err("invalid comparison operator"))
.ok_or_else(|| #pyo3_path::exceptions::PyValueError::new_err1("invalid comparison operator"))
},
ctx
),
Ty::PySsizeT => {
let ty = arg.ty();
extract_error_mode.handle_error(
quote! {
::std::convert::TryInto::<#ty>::try_into(#ident).map_err(|e| #pyo3_path::exceptions::PyValueError::new_err(e.to_string()))
::std::convert::TryInto::<#ty>::try_into(#ident).map_err(|e| #pyo3_path::exceptions::PyValueError::new_err1(e.to_string()))
},
ctx
)
Expand Down
4 changes: 2 additions & 2 deletions pytests/src/awaitable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ impl IterAwaitable {
fn __next__(&mut self, py: Python<'_>) -> PyResult<PyObject> {
match self.result.take() {
Some(res) => match res {
Ok(v) => Err(PyStopIteration::new_err(v)),
Ok(v) => Err(PyStopIteration::new_err1(v)),
Err(err) => Err(err),
},
_ => Ok(py.None()),
Expand Down Expand Up @@ -70,7 +70,7 @@ impl FutureAwaitable {
fn __next__(mut pyself: PyRefMut<'_, Self>) -> PyResult<PyRefMut<'_, Self>> {
match pyself.result {
Some(_) => match pyself.result.take().unwrap() {
Ok(v) => Err(PyStopIteration::new_err(v)),
Ok(v) => Err(PyStopIteration::new_err1(v)),
Err(err) => Err(err),
},
_ => Ok(pyself),
Expand Down
2 changes: 1 addition & 1 deletion pytests/src/dict_iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ impl DictSize {
if seen == self.expected {
Ok(seen)
} else {
Err(PyErr::new::<PyRuntimeError, _>(format!(
Err(PyErr::new1::<PyRuntimeError, _>(format!(
"Expected {} iterations - performed {}",
self.expected, seen
)))
Expand Down
4 changes: 2 additions & 2 deletions pytests/src/pyclasses.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ impl PyClassIter {
self.count += 1;
Ok(self.count)
} else {
Err(PyStopIteration::new_err("Ended"))
Err(PyStopIteration::new_err1("Ended"))
}
}
}
Expand All @@ -54,7 +54,7 @@ impl AssertingBaseClass {
#[classmethod]
fn new(cls: &Bound<'_, PyType>, expected_type: Bound<'_, PyType>) -> PyResult<Self> {
if !cls.is(&expected_type) {
return Err(PyValueError::new_err(format!(
return Err(PyValueError::new_err1(format!(
"{:?} != {:?}",
cls, expected_type
)));
Expand Down
14 changes: 7 additions & 7 deletions src/buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,16 +206,16 @@ impl<T: Element> PyBuffer<T> {
let buf = PyBuffer(Pin::from(buf), PhantomData);

if buf.0.shape.is_null() {
Err(PyBufferError::new_err("shape is null"))
Err(PyBufferError::new_err1("shape is null"))
} else if buf.0.strides.is_null() {
Err(PyBufferError::new_err("strides is null"))
Err(PyBufferError::new_err1("strides is null"))
} else if mem::size_of::<T>() != buf.item_size() || !T::is_compatible_format(buf.format()) {
Err(PyBufferError::new_err(format!(
Err(PyBufferError::new_err1(format!(
"buffer contents are not compatible with {}",
std::any::type_name::<T>()
)))
} else if buf.0.buf.align_offset(mem::align_of::<T>()) != 0 {
Err(PyBufferError::new_err(format!(
Err(PyBufferError::new_err1(format!(
"buffer contents are insufficiently aligned for {}",
std::any::type_name::<T>()
)))
Expand Down Expand Up @@ -476,7 +476,7 @@ impl<T: Element> PyBuffer<T> {

fn _copy_to_slice(&self, py: Python<'_>, target: &mut [T], fort: u8) -> PyResult<()> {
if mem::size_of_val(target) != self.len_bytes() {
return Err(PyBufferError::new_err(format!(
return Err(PyBufferError::new_err1(format!(
"slice to copy to (of length {}) does not match buffer length of {}",
target.len(),
self.item_count()
Expand Down Expand Up @@ -568,9 +568,9 @@ impl<T: Element> PyBuffer<T> {

fn _copy_from_slice(&self, py: Python<'_>, source: &[T], fort: u8) -> PyResult<()> {
if self.readonly() {
return Err(PyBufferError::new_err("cannot write to read-only buffer"));
return Err(PyBufferError::new_err1("cannot write to read-only buffer"));
} else if mem::size_of_val(source) != self.len_bytes() {
return Err(PyBufferError::new_err(format!(
return Err(PyBufferError::new_err1(format!(
"slice to copy from (of length {}) does not match buffer length of {}",
source.len(),
self.item_count()
Expand Down
2 changes: 1 addition & 1 deletion src/callback.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ impl IntoPyCallbackOutput<()> for () {
impl IntoPyCallbackOutput<ffi::Py_ssize_t> for usize {
#[inline]
fn convert(self, _py: Python<'_>) -> PyResult<ffi::Py_ssize_t> {
self.try_into().map_err(|_err| PyOverflowError::new_err(()))
self.try_into().map_err(|_err| PyOverflowError::new_err0())
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/conversions/anyhow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ impl From<anyhow::Error> for PyErr {
Err(error) => error,
};
}
PyRuntimeError::new_err(format!("{:?}", error))
PyRuntimeError::new_err1(format!("{:?}", error))
}
}

Expand Down Expand Up @@ -175,7 +175,7 @@ mod test_anyhow {

#[test]
fn test_pyo3_unwrap_simple_err() {
let origin_exc = PyValueError::new_err("Value Error");
let origin_exc = PyValueError::new_err0();
let err: anyhow::Error = origin_exc.into();
let converted: PyErr = err.into();
assert!(Python::with_gil(
Expand All @@ -184,7 +184,7 @@ mod test_anyhow {
}
#[test]
fn test_pyo3_unwrap_complex_err() {
let origin_exc = PyValueError::new_err("Value Error");
let origin_exc = PyValueError::new_err0();
let mut err: anyhow::Error = origin_exc.into();
err = err.context("Context");
let converted: PyErr = err.into();
Expand Down
Loading

0 comments on commit 92b2dc7

Please sign in to comment.