Skip to content
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

Upgrade to PyO3 0.23 #137

Merged
merged 4 commits into from
Nov 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading