Skip to content

Commit

Permalink
minimal upgrade to PyO3 0.23 (ignoring deprecations) (#1556)
Browse files Browse the repository at this point in the history
  • Loading branch information
davidhewitt authored Nov 26, 2024
1 parent 334e3df commit 5c96bec
Show file tree
Hide file tree
Showing 16 changed files with 107 additions and 78 deletions.
24 changes: 12 additions & 12 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ rust-version = "1.75"
[dependencies]
# TODO it would be very nice to remove the "py-clone" feature as it can panic,
# but needs a bit of work to make sure it's not used in the codebase
pyo3 = { version = "0.22.6", features = ["generate-import-lib", "num-bigint", "py-clone"] }
pyo3 = { version = "0.23.2", features = ["generate-import-lib", "num-bigint", "py-clone"] }
regex = "1.11.1"
strum = { version = "0.26.3", features = ["derive"] }
strum_macros = "0.26.4"
Expand All @@ -46,7 +46,7 @@ base64 = "0.22.1"
num-bigint = "0.4.6"
python3-dll-a = "0.2.10"
uuid = "1.11.0"
jiter = { version = "0.7.1", features = ["python"] }
jiter = { version = "0.8.0", features = ["python"] }
hex = "0.4.3"

[lib]
Expand Down Expand Up @@ -74,12 +74,12 @@ debug = true
strip = false

[dev-dependencies]
pyo3 = { version = "0.22.6", features = ["auto-initialize"] }
pyo3 = { version = "0.23.2", features = ["auto-initialize"] }

[build-dependencies]
version_check = "0.9.5"
# used where logic has to be version/distribution specific, e.g. pypy
pyo3-build-config = { version = "0.22.6" }
pyo3-build-config = { version = "0.23.2" }

[lints.clippy]
dbg_macro = "warn"
Expand Down
1 change: 1 addition & 0 deletions benches/main.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#![feature(test)]
#![allow(deprecated)] // FIXME: just used during upgrading PyO3 to 0.23

extern crate test;

Expand Down
2 changes: 1 addition & 1 deletion src/errors/validation_exception.rs
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,7 @@ impl ValidationError {
let args = (
borrow.title.bind(py),
borrow.errors(py, include_url_env(py), true, true)?,
borrow.input_type.into_py(py),
borrow.input_type.into_pyobject(py)?,
borrow.hide_input,
)
.into_py(slf.py());
Expand Down
2 changes: 1 addition & 1 deletion src/input/datetime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -527,7 +527,7 @@ impl TzInfo {
}

#[allow(unused_variables)]
fn dst(&self, dt: &Bound<'_, PyAny>) -> Option<&PyDelta> {
fn dst(&self, dt: &Bound<'_, PyAny>) -> Option<Bound<'_, PyDelta>> {
None
}

Expand Down
21 changes: 13 additions & 8 deletions src/input/input_abstract.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use std::convert::Infallible;
use std::fmt;

use pyo3::exceptions::PyValueError;
use pyo3::types::{PyDict, PyList};
use pyo3::types::{PyDict, PyList, PyString};
use pyo3::{intern, prelude::*};

use crate::errors::{ErrorTypeDefaults, InputValue, LocItem, ValError, ValResult};
Expand All @@ -20,13 +21,17 @@ pub enum InputType {
String,
}

impl IntoPy<PyObject> for InputType {
fn into_py(self, py: Python<'_>) -> PyObject {
match self {
Self::Json => intern!(py, "json").into_py(py),
Self::Python => intern!(py, "python").into_py(py),
Self::String => intern!(py, "string").into_py(py),
}
impl<'py> IntoPyObject<'py> for InputType {
type Target = PyString;
type Output = Bound<'py, PyString>;
type Error = Infallible;

fn into_pyobject(self, py: Python<'_>) -> Result<Bound<'_, PyString>, Infallible> {
Ok(match self {
Self::Json => intern!(py, "json").clone(),
Self::Python => intern!(py, "python").clone(),
Self::String => intern!(py, "string").clone(),
})
}
}

Expand Down
8 changes: 3 additions & 5 deletions src/input/return_enums.rs
Original file line number Diff line number Diff line change
Expand Up @@ -278,18 +278,16 @@ pub(crate) fn iterate_mapping_items<'a, 'py>(
.items()
.map_err(|e| mapping_err(e, py, input))?
.iter()
.map_err(|e| mapping_err(e, py, input))?
.map(move |item| match item {
Ok(item) => item.extract().map_err(|_| {
.map(move |item| {
item.extract().map_err(|_| {
ValError::new(
ErrorType::MappingType {
error: MAPPING_TUPLE_ERROR.into(),
context: None,
},
input,
)
}),
Err(e) => Err(mapping_err(e, py, input)),
})
});
Ok(iterator)
}
Expand Down
1 change: 1 addition & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#![cfg_attr(has_coverage_attribute, feature(coverage_attribute))]
#![allow(deprecated)] // FIXME: just used during upgrading PyO3 to 0.23

extern crate core;

Expand Down
17 changes: 11 additions & 6 deletions src/lookup_key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use std::fmt;
use pyo3::exceptions::{PyAttributeError, PyTypeError};
use pyo3::prelude::*;
use pyo3::types::{PyDict, PyList, PyMapping, PyString};
use pyo3::IntoPyObjectExt;

use jiter::{JsonObject, JsonValue};

Expand Down Expand Up @@ -407,14 +408,18 @@ impl fmt::Display for PathItem {
}
}

impl ToPyObject for PathItem {
fn to_object(&self, py: Python<'_>) -> PyObject {
impl<'py> IntoPyObject<'py> for &'_ PathItem {
type Target = PyAny;
type Output = Bound<'py, PyAny>;
type Error = PyErr;

fn into_pyobject(self, py: Python<'py>) -> Result<Self::Output, Self::Error> {
match self {
Self::S(_, val) => val.to_object(py),
Self::Pos(val) => val.to_object(py),
Self::Neg(val) => {
PathItem::S(_, val) => Ok(val.bind(py).clone().into_any()),
PathItem::Pos(val) => val.into_bound_py_any(py),
PathItem::Neg(val) => {
let neg_value = -(*val as i64);
neg_value.to_object(py)
neg_value.into_bound_py_any(py)
}
}
}
Expand Down
62 changes: 38 additions & 24 deletions src/serializers/extra.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use std::cell::RefCell;
use std::fmt;
use std::sync::Mutex;

use pyo3::exceptions::{PyTypeError, PyValueError};
use pyo3::intern;
Expand Down Expand Up @@ -370,18 +370,27 @@ impl From<bool> for WarningsMode {
}
}

#[derive(Clone)]
#[cfg_attr(debug_assertions, derive(Debug))]
pub(crate) struct CollectWarnings {
mode: WarningsMode,
warnings: RefCell<Option<Vec<String>>>,
// FIXME: mutex is to satisfy PyO3 0.23, we should be able to refactor this away
warnings: Mutex<Vec<String>>,
}

impl Clone for CollectWarnings {
fn clone(&self) -> Self {
Self {
mode: self.mode,
warnings: Mutex::new(self.warnings.lock().expect("lock poisoned").clone()),
}
}
}

impl CollectWarnings {
pub(crate) fn new(mode: WarningsMode) -> Self {
Self {
mode,
warnings: RefCell::new(None),
warnings: Mutex::new(Vec::new()),
}
}

Expand Down Expand Up @@ -447,41 +456,46 @@ impl CollectWarnings {
}

fn add_warning(&self, message: String) {
let mut op_warnings = self.warnings.borrow_mut();
if let Some(ref mut warnings) = *op_warnings {
warnings.push(message);
} else {
*op_warnings = Some(vec![message]);
}
self.warnings.lock().expect("lock poisoned").push(message);
}

pub fn final_check(&self, py: Python) -> PyResult<()> {
if self.mode == WarningsMode::None {
return Ok(());
}
match *self.warnings.borrow() {
Some(ref warnings) => {
let message = format!("Pydantic serializer warnings:\n {}", warnings.join("\n "));
if self.mode == WarningsMode::Warn {
let user_warning_type = py.import_bound("builtins")?.getattr("UserWarning")?;
PyErr::warn_bound(py, &user_warning_type, &message, 0)
} else {
Err(PydanticSerializationError::new_err(message))
}
}
_ => Ok(()),
let warnings = self.warnings.lock().expect("lock poisoned");

if warnings.is_empty() {
return Ok(());
}

let message = format!("Pydantic serializer warnings:\n {}", warnings.join("\n "));
if self.mode == WarningsMode::Warn {
let user_warning_type = py.import_bound("builtins")?.getattr("UserWarning")?;
PyErr::warn_bound(py, &user_warning_type, &message, 0)
} else {
Err(PydanticSerializationError::new_err(message))
}
}
}

#[derive(Default, Clone)]
#[derive(Default)]
#[cfg_attr(debug_assertions, derive(Debug))]
pub struct SerRecursionState {
guard: RefCell<RecursionState>,
// FIXME: mutex is to satisfy PyO3 0.23, we should be able to refactor this away
guard: Mutex<RecursionState>,
}

impl Clone for SerRecursionState {
fn clone(&self) -> Self {
Self {
guard: Mutex::new(self.guard.lock().expect("lock poisoned").clone()),
}
}
}

impl ContainsRecursionState for &'_ Extra<'_> {
fn access_recursion_state<R>(&mut self, f: impl FnOnce(&mut RecursionState) -> R) -> R {
f(&mut self.rec_guard.guard.borrow_mut())
f(&mut self.rec_guard.guard.lock().expect("lock poisoned"))
}
}
12 changes: 6 additions & 6 deletions src/serializers/filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ impl SchemaFilter<isize> {

pub fn key_filter<'py>(
&self,
key: &Bound<'_, PyAny>,
key: &Bound<'py, PyAny>,
include: Option<&Bound<'py, PyAny>>,
exclude: Option<&Bound<'py, PyAny>>,
) -> PyResult<NextFilters<'py>> {
Expand All @@ -152,7 +152,7 @@ trait FilterLogic<T: Eq + Copy> {
/// 2. or include it, in which case, what values of `include` and `exclude` should be passed to it
fn filter<'py>(
&self,
py_key: impl ToPyObject + Copy,
py_key: impl IntoPyObject<'py> + Copy,
int_key: T,
include: Option<&Bound<'py, PyAny>>,
exclude: Option<&Bound<'py, PyAny>>,
Expand Down Expand Up @@ -266,7 +266,7 @@ impl AnyFilter {

pub fn key_filter<'py>(
&self,
key: &Bound<'_, PyAny>,
key: &Bound<'py, PyAny>,
include: Option<&Bound<'py, PyAny>>,
exclude: Option<&Bound<'py, PyAny>>,
) -> PyResult<NextFilters<'py>> {
Expand All @@ -289,11 +289,11 @@ impl AnyFilter {

/// if a `__contains__` method exists, call it with the key and `__all__`, and return the result
/// if it doesn't exist, or calling it fails (e.g. it's not a function), return `None`
fn check_contains(obj: &Bound<'_, PyAny>, py_key: impl ToPyObject + Copy) -> PyResult<Option<bool>> {
fn check_contains<'py>(obj: &Bound<'py, PyAny>, py_key: impl IntoPyObject<'py> + Copy) -> PyResult<Option<bool>> {
let py = obj.py();
match obj.getattr(intern!(py, "__contains__")) {
Ok(contains_method) => {
if let Ok(result) = contains_method.call1((py_key.to_object(py),)) {
if let Ok(result) = contains_method.call1((py_key,)) {
Ok(Some(
result.is_truthy()? || contains_method.call1((intern!(py, "__all__"),))?.is_truthy()?,
))
Expand Down Expand Up @@ -330,7 +330,7 @@ fn is_ellipsis_like(v: &Bound<'_, PyAny>) -> bool {
/// lookup the dict, for the key and "__all__" key, and merge them following the same rules as pydantic V1
fn merge_all_value<'py>(
dict: &Bound<'py, PyDict>,
py_key: impl ToPyObject + Copy,
py_key: impl IntoPyObject<'py> + Copy,
) -> PyResult<Option<Bound<'py, PyAny>>> {
let op_item_value = dict.get_item(py_key)?;
let op_all_value = dict.get_item(intern!(dict.py(), "__all__"))?;
Expand Down
Loading

0 comments on commit 5c96bec

Please sign in to comment.