Skip to content

Commit

Permalink
Upgrade to PyO3 0.23 (#137)
Browse files Browse the repository at this point in the history
  • Loading branch information
davidhewitt authored Nov 15, 2024
1 parent 1fbdb1e commit 9bde9e1
Show file tree
Hide file tree
Showing 7 changed files with 108 additions and 34 deletions.
4 changes: 2 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,5 @@ inherits = "release"
debug = true

[workspace.dependencies]
pyo3 = { version = "0.22.0" }
pyo3-build-config = { version = "0.22.0" }
pyo3 = { version = "0.23" }
pyo3-build-config = { version = "0.23" }
5 changes: 5 additions & 0 deletions crates/jiter/src/number_decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
use num_bigint::BigInt;
#[cfg(feature = "num-bigint")]
use num_traits::cast::ToPrimitive;
#[cfg(feature = "python")]
use pyo3::{IntoPyObject, IntoPyObjectRef};

use std::ops::Range;

Expand All @@ -17,6 +19,7 @@ pub trait AbstractNumberDecoder {

/// A number that can be either an [i64] or a [BigInt](num_bigint::BigInt)
#[derive(Debug, Clone, PartialEq)]
#[cfg_attr(feature = "python", derive(IntoPyObject, IntoPyObjectRef))]
pub enum NumberInt {
Int(i64),
#[cfg(feature = "num-bigint")]
Expand Down Expand Up @@ -113,12 +116,14 @@ impl AbstractNumberDecoder for NumberFloat {

/// A number that can be either a [NumberInt] or an [f64]
#[derive(Debug, Clone, PartialEq)]
#[cfg_attr(feature = "python", derive(IntoPyObject, IntoPyObjectRef))]
pub enum NumberAny {
Int(NumberInt),
Float(f64),
}

#[cfg(feature = "python")]
#[allow(deprecated)] // kept around for downstream sake
impl pyo3::ToPyObject for NumberAny {
fn to_object(&self, py: pyo3::Python<'_>) -> pyo3::PyObject {
match self {
Expand Down
4 changes: 1 addition & 3 deletions crates/jiter/src/py_lossless_float.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,5 @@ impl LosslessFloat {
static DECIMAL_TYPE: GILOnceCell<Py<PyType>> = GILOnceCell::new();

pub fn get_decimal_type(py: Python) -> PyResult<&Bound<'_, PyType>> {
DECIMAL_TYPE
.get_or_try_init(py, || py.import_bound("decimal")?.getattr("Decimal")?.extract())
.map(|t| t.bind(py))
DECIMAL_TYPE.import(py, "decimal", "Decimal")
}
2 changes: 1 addition & 1 deletion crates/jiter/src/py_string_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ pub fn pystring_fast_new<'py>(py: Python<'py>, s: &str, ascii_only: bool) -> Bou
if ascii_only {
unsafe { pystring_ascii_new(py, s) }
} else {
PyString::new_bound(py, s)
PyString::new(py, s)
}
}

Expand Down
42 changes: 27 additions & 15 deletions crates/jiter/src/python.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use pyo3::exceptions::{PyTypeError, PyValueError};
use pyo3::ffi;
use pyo3::prelude::*;
use pyo3::types::{PyBool, PyDict, PyList, PyString};
use pyo3::ToPyObject;

use smallvec::SmallVec;

Expand Down Expand Up @@ -129,11 +128,11 @@ impl<'j, StringCache: StringMaybeCache, KeyCheck: MaybeKeyCheck, ParseNumber: Ma
}
Peek::True => {
self.parser.consume_true()?;
Ok(true.to_object(py).into_bound(py))
Ok(PyBool::new(py, true).to_owned().into_any())
}
Peek::False => {
self.parser.consume_false()?;
Ok(false.to_object(py).into_bound(py))
Ok(PyBool::new(py, false).to_owned().into_any())
}
Peek::String => {
let s = self
Expand All @@ -145,7 +144,7 @@ impl<'j, StringCache: StringMaybeCache, KeyCheck: MaybeKeyCheck, ParseNumber: Ma
let peek_first = match self.parser.array_first() {
Ok(Some(peek)) => peek,
Err(e) if !self._allow_partial_err(&e) => return Err(e),
Ok(None) | Err(_) => return Ok(PyList::empty_bound(py).into_any()),
Ok(None) | Err(_) => return Ok(PyList::empty(py).into_any()),
};

let mut vec: SmallVec<[Bound<'_, PyAny>; 8]> = SmallVec::with_capacity(8);
Expand All @@ -155,10 +154,12 @@ impl<'j, StringCache: StringMaybeCache, KeyCheck: MaybeKeyCheck, ParseNumber: Ma
}
}

Ok(PyList::new_bound(py, vec).into_any())
Ok(PyList::new(py, vec)
.map_err(|e| py_err_to_json_err(&e, self.parser.index))?
.into_any())
}
Peek::Object => {
let dict = PyDict::new_bound(py);
let dict = PyDict::new(py);
if let Err(e) = self._parse_object(py, &dict) {
if !self._allow_partial_err(&e) {
return Err(e);
Expand Down Expand Up @@ -298,7 +299,10 @@ impl MaybeParseNumber for ParseNumberLossy {
allow_inf_nan: bool,
) -> JsonResult<Bound<'py, PyAny>> {
match parser.consume_number::<NumberAny>(peek.into_inner(), allow_inf_nan) {
Ok(number) => Ok(number.to_object(py).into_bound(py)),
Ok(number) => Ok(number
.into_pyobject(py)
.map_err(|e| py_err_to_json_err(&e, parser.index))?
.into_any()),
Err(e) => {
if !peek.is_num() {
Err(json_error!(ExpectedSomeValue, parser.index))
Expand All @@ -325,11 +329,15 @@ impl MaybeParseNumber for ParseNumberLossless {
let obj = if number_range.is_int {
NumberAny::decode(bytes, 0, peek.into_inner(), allow_inf_nan)?
.0
.to_object(py)
.into_pyobject(py)
.map_err(|e| py_err_to_json_err(&e, parser.index))?
} else {
LosslessFloat::new_unchecked(bytes.to_vec()).into_py(py)
LosslessFloat::new_unchecked(bytes.to_vec())
.into_pyobject(py)
.map_err(|e| py_err_to_json_err(&e, parser.index))?
.into_any()
};
Ok(obj.into_bound(py))
Ok(obj)
}
Err(e) => {
if !peek.is_num() {
Expand Down Expand Up @@ -357,17 +365,17 @@ impl MaybeParseNumber for ParseNumberDecimal {
if number_range.is_int {
let obj = NumberAny::decode(bytes, 0, peek.into_inner(), allow_inf_nan)?
.0
.to_object(py);
Ok(obj.into_bound(py))
.into_pyobject(py)
.map_err(|e| py_err_to_json_err(&e, parser.index))?;
Ok(obj.into_any())
} else {
let decimal_type = get_decimal_type(py)
.map_err(|e| JsonError::new(JsonErrorType::InternalError(e.to_string()), parser.index))?;
let decimal_type = get_decimal_type(py).map_err(|e| py_err_to_json_err(&e, parser.index))?;
// SAFETY: NumberRange::decode has already confirmed that bytes are a valid JSON number,
// and therefore valid str
let float_str = unsafe { std::str::from_utf8_unchecked(bytes) };
decimal_type
.call1((float_str,))
.map_err(|e| JsonError::new(JsonErrorType::InternalError(e.to_string()), parser.index))
.map_err(|e| py_err_to_json_err(&e, parser.index))
}
}
Err(e) => {
Expand All @@ -380,3 +388,7 @@ impl MaybeParseNumber for ParseNumberDecimal {
}
}
}

fn py_err_to_json_err(e: &PyErr, index: usize) -> JsonError {
JsonError::new(JsonErrorType::InternalError(e.to_string()), index)
}
57 changes: 57 additions & 0 deletions crates/jiter/src/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ pub type JsonArray<'s> = Arc<SmallVec<[JsonValue<'s>; 8]>>;
pub type JsonObject<'s> = Arc<LazyIndexMap<Cow<'s, str>, JsonValue<'s>>>;

#[cfg(feature = "python")]
#[allow(deprecated)] // keeping around for sake of allowing downstream to migrate
impl pyo3::ToPyObject for JsonValue<'_> {
fn to_object(&self, py: pyo3::Python<'_>) -> pyo3::PyObject {
use pyo3::prelude::*;
Expand All @@ -53,6 +54,62 @@ impl pyo3::ToPyObject for JsonValue<'_> {
}
}

#[cfg(feature = "python")]
impl<'py> pyo3::IntoPyObject<'py> for JsonValue<'_> {
type Error = pyo3::PyErr;
type Target = pyo3::PyAny;
type Output = pyo3::Bound<'py, pyo3::PyAny>;

fn into_pyobject(self, py: pyo3::Python<'py>) -> Result<Self::Output, Self::Error> {
use pyo3::prelude::*;
match self {
Self::Null => Ok(py.None().into_pyobject(py)?),
Self::Bool(b) => Ok(b.into_pyobject(py)?.to_owned().into_any()),
Self::Int(i) => Ok(i.into_pyobject(py)?.into_any()),
#[cfg(feature = "num-bigint")]
Self::BigInt(b) => Ok(b.into_pyobject(py)?.into_any()),
Self::Float(f) => Ok(f.into_pyobject(py)?.into_any()),
Self::Str(s) => Ok(s.into_pyobject(py)?.into_any()),
Self::Array(v) => Ok(pyo3::types::PyList::new(py, v.iter())?.into_any()),
Self::Object(o) => {
let dict = pyo3::types::PyDict::new(py);
for (k, v) in o.iter() {
dict.set_item(k, v).unwrap();
}
Ok(dict.into_any())
}
}
}
}

#[cfg(feature = "python")]
impl<'py> pyo3::IntoPyObject<'py> for &'_ JsonValue<'_> {
type Error = pyo3::PyErr;
type Target = pyo3::PyAny;
type Output = pyo3::Bound<'py, pyo3::PyAny>;

fn into_pyobject(self, py: pyo3::Python<'py>) -> Result<Self::Output, Self::Error> {
use pyo3::prelude::*;
match self {
JsonValue::Null => Ok(py.None().into_pyobject(py)?),
JsonValue::Bool(b) => Ok(b.into_pyobject(py)?.to_owned().into_any()),
JsonValue::Int(i) => Ok(i.into_pyobject(py)?.into_any()),
#[cfg(feature = "num-bigint")]
JsonValue::BigInt(b) => Ok(b.into_pyobject(py)?.into_any()),
JsonValue::Float(f) => Ok(f.into_pyobject(py)?.into_any()),
JsonValue::Str(s) => Ok(s.into_pyobject(py)?.into_any()),
JsonValue::Array(v) => Ok(pyo3::types::PyList::new(py, v.iter())?.into_any()),
JsonValue::Object(o) => {
let dict = pyo3::types::PyDict::new(py);
for (k, v) in o.iter() {
dict.set_item(k, v).unwrap();
}
Ok(dict.into_any())
}
}
}
}

impl<'j> JsonValue<'j> {
/// Parse a JSON enum from a byte slice, returning a borrowed version of the enum - e.g. strings can be
/// references into the original byte slice.
Expand Down
28 changes: 15 additions & 13 deletions crates/jiter/tests/python.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ fn test_to_py_object_numeric() {
)
.unwrap();
Python::with_gil(|py| {
let python_value = value.to_object(py);
let string = python_value.bind(py).to_string();
let python_value = value.into_pyobject(py).unwrap();
let string = python_value.to_string();
assert_eq!(
string,
"{'int': 1, 'bigint': 123456789012345678901234567890, 'float': 1.2}"
Expand All @@ -28,38 +28,40 @@ fn test_to_py_object_other() {
)
.unwrap();
Python::with_gil(|py| {
let python_value = value.to_object(py);
let string = python_value.bind(py).to_string();
let python_value = value.into_pyobject(py).unwrap();
let string = python_value.to_string();
assert_eq!(string, "['string', '£', True, False, None, nan, inf, -inf]");
})
}

#[test]
fn test_cache_into() {
Python::with_gil(|py| {
let c: StringCacheMode = true.to_object(py).extract(py).unwrap();
let c: StringCacheMode = true.into_pyobject(py).unwrap().extract().unwrap();
assert!(matches!(c, StringCacheMode::All));

let c: StringCacheMode = false.to_object(py).extract(py).unwrap();
let c: StringCacheMode = false.into_pyobject(py).unwrap().extract().unwrap();
assert!(matches!(c, StringCacheMode::None));

let c: StringCacheMode = PyString::new_bound(py, "all").extract().unwrap();
let c: StringCacheMode = PyString::new(py, "all").extract().unwrap();
assert!(matches!(c, StringCacheMode::All));

let c: StringCacheMode = PyString::new_bound(py, "keys").extract().unwrap();
let c: StringCacheMode = PyString::new(py, "keys").extract().unwrap();
assert!(matches!(c, StringCacheMode::Keys));

let c: StringCacheMode = PyString::new_bound(py, "none").extract().unwrap();
let c: StringCacheMode = PyString::new(py, "none").extract().unwrap();
assert!(matches!(c, StringCacheMode::None));

let e = PyString::new_bound(py, "wrong")
.extract::<StringCacheMode>()
.unwrap_err();
let e = PyString::new(py, "wrong").extract::<StringCacheMode>().unwrap_err();
assert_eq!(
e.to_string(),
"ValueError: Invalid string cache mode, should be `'all'`, '`keys`', `'none`' or a `bool`"
);
let e = 123.to_object(py).extract::<StringCacheMode>(py).unwrap_err();
let e = 123i32
.into_pyobject(py)
.unwrap()
.extract::<StringCacheMode>()
.unwrap_err();
assert_eq!(
e.to_string(),
"TypeError: Invalid string cache mode, should be `'all'`, '`keys`', `'none`' or a `bool`"
Expand Down

0 comments on commit 9bde9e1

Please sign in to comment.