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

Add docs + slight refactor for num-complex and num-bigint dependencies #1670

Merged
merged 10 commits into from
Jun 11, 2021
2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ proptest = { version = "0.10.1", default-features = false, features = ["std"] }
# features needed to run the PyO3 test suite
pyo3 = { path = ".", default-features = false, features = ["macros", "auto-initialize"] }
serde_json = "1.0.61"
# needed for num-complex doctest
nalgebra = "0.27.1"

[build-dependencies]
pyo3-build-config = { path = "pyo3-build-config", version = "=0.14.0-alpha.0" }
Expand Down
8 changes: 8 additions & 0 deletions guide/src/features.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,14 @@ The `nightly` feature needs the nightly Rust compiler. This allows PyO3 to use R
- `FromPyObject` for `Vec` and `[T;N]` can perform a `memcpy` when the object supports the Python buffer protocol.
- `ToBorrowedObject` can skip a reference count increase when the provided object is a Python native type.

### `num-bigint`

This feature adds a dependency on [num-bigint](https://docs.rs/num-bigint) and enables conversions into its [`BigInt`](https://docs.rs/num-bigint/latest/num_bigint/struct.BigInt.html) and [`BigUint`](https://docs.rs/num-bigint/latest/num_bigint/struct.BigUInt.html) types.

### `num-complex`

This feature adds a dependency on [num-complex](https://docs.rs/num-complex) and enables conversions into its [`Complex`](https://docs.rs/num-complex/latest/num_complex/struct.Complex.html) type.

### `serde`

The `serde` feature enables (de)serialization of Py<T> objects via [serde](https://serde.rs/).
Expand Down
8 changes: 6 additions & 2 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,12 +80,12 @@
//! [`#[pyclass]`](crate::proc_macro::pyclass). This adds a dependency on the
//! [`inventory`](https://docs.rs/inventory) crate, which is not supported on all platforms.
//
//! - `num-bigint`: Enables conversions between Python objects and
//! - [`num-bigint`](crate::num_bigint): Enables conversions between Python objects and
//! [num-bigint](https://docs.rs/num-bigint)'s
//! [`BigInt`](https://docs.rs/num-bigint/latest/num_bigint/struct.BigInt.html) and
//! [`BigUint`](https://docs.rs/num-bigint/latest/num_bigint/struct.BigUint.html) types.
//
//! - `num-complex`: Enables conversions between Python objects and
//! - [`num-complex`](crate::num_complex): Enables conversions between Python objects and
//! [num-complex](https://docs.rs/num-complex)'s
//! [`Complex`](https://docs.rs/num-complex/latest/num_complex/struct.Complex.html) type.
//
Expand Down Expand Up @@ -298,6 +298,10 @@ mod python;
pub mod type_object;
pub mod types;

pub mod num_bigint;

pub mod num_complex;

#[cfg_attr(docsrs, doc(cfg(feature = "serde")))]
#[cfg(feature = "serde")]
pub mod serde;
Expand Down
288 changes: 288 additions & 0 deletions src/num_bigint.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,288 @@
// Copyright (c) 2017-present PyO3 Project and Contributors
//
// based on Daniel Grunwald's https://github.com/dgrunwald/rust-cpython

#![cfg(all(feature = "num-bigint", not(any(Py_LIMITED_API, PyPy))))]
#![cfg_attr(
docsrs,
doc(cfg(all(feature = "num-bigint", not(any(Py_LIMITED_API, PyPy)))))
)]
//! Conversions to and from [num-bigint](https://docs.rs/num-bigint)’s [`BigInt`] and [`BigUint`] types.
//!
//! This is useful for converting Python integers when they may not fit in Rust's built-in integer types.
//!
//! # Setup
//!
//! To use this feature, add this to your **`Cargo.toml`**:
//!
//! ```toml
//! [dependencies]
//! num-bigint = "0.4"
//! pyo3 = { version = "0.14", features = ["num-bigint"] }
//! ```
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed this is useful to have but I worry we'll forget to update the versions...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there is a way to auto update it :/ This is definitely going to be an annoying maintenance burden going forward.

I've seen other crates document this like:

[dependencies]
# replace * with the current pyo3 version
pyo3 = { version = "*", features = ["num-bigint"] }

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 That seems like a really good idea. Could also say "check pyo3's Cargo.toml" for supported num-bigint version?

... or we could do something fun with an mdbook preprocessor to parse Cargo.toml and put the supported num-bigint version in the guide in a table 🙈

Copy link
Member Author

@mejrs mejrs Jun 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or we could do something fun with an mdbook preprocessor to parse Cargo.toml and put the supported num-bigint version in the guide in a table 🙈

Can we do something dynamically in the docs? I'd really prefer something that can be straight up copy-pasted.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not aware of similar tricks to do dynamic docs, however if you want to go for something dynamic in the guide you might find the existing preprocessor I wrote useful: https://github.com/PyO3/pyo3/blob/main/guide/pyo3_version.py

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we have other locations where the current version must be substituted, I think the best way is a small (Python) script that replaces everything at once. This can then be run simply as part of the release process.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried a couple of things but didn't find anything that didn't feel like a gross hack. So I've just taken the * approach as above.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the "gross hack" with my suggestion?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not, really. I figured out a proc macro that does it by just search+replacing text in a module, but it doesn't work in lib.rs and messes up the [src] links (which is pretty gross).

I don't like the idea of adding things to the release process for something that's at best "nice to have". It's not that big of a deal on its own but these things do add up.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TBH I would be happy to have a python script to automate releasing (at the moment it's done manually by me), but i think that's best in a separate PR.

//!
//! Note that you must use compatible versions of num-bigint and PyO3.
//! The required num-bigint version may vary with older versions of PyO3.
//!
//! ## Examples
//!
//! Using [`BigInt`] to correctly increment an arbitrary precision integer.
//! This is not possible with Rust's native integers if the Python integer is too large,
//! in which case it will fail its conversion and raise `OverflowError`.
//!
//! ```rust
//! use num_bigint::BigInt;
//! use pyo3::prelude::*;
//! use pyo3::wrap_pyfunction;
//!
//! #[pyfunction]
//! fn add_one(n: BigInt) -> BigInt {
//! n + 1
//! }
//!
//! #[pymodule]
//! fn my_module(_py: Python, m: &PyModule) -> PyResult<()> {
//! m.add_function(wrap_pyfunction!(add_one, m)?)?;
//! Ok(())
//! }
//! ```
//!
//! Python code:
//! ```python
//! from my_module import add_one
//!
//! n = 1 << 1337
//! value = add_one(n)
//!
//! assert n + 1 == value
//! ```

use crate::{
err, ffi, types::*, AsPyPointer, FromPyObject, IntoPy, Py, PyAny, PyErr, PyNativeType,
PyObject, PyResult, Python, ToPyObject,
};

use num_bigint::{BigInt, BigUint};
use std::os::raw::{c_int, c_uchar};

#[cfg(not(all(windows, PyPy)))]
unsafe fn extract(ob: &PyLong, buffer: &mut [c_uchar], is_signed: c_int) -> PyResult<()> {
err::error_on_minusone(
ob.py(),
ffi::_PyLong_AsByteArray(
ob.as_ptr() as *mut ffi::PyLongObject,
buffer.as_mut_ptr(),
buffer.len(),
1,
is_signed,
),
)
}

macro_rules! bigint_conversion {
($rust_ty: ty, $is_signed: expr, $to_bytes: path, $from_bytes: path) => {
impl ToPyObject for $rust_ty {
fn to_object(&self, py: Python) -> PyObject {
unsafe {
let bytes = $to_bytes(self);
let obj = ffi::_PyLong_FromByteArray(
bytes.as_ptr() as *const c_uchar,
bytes.len(),
1,
$is_signed,
);
PyObject::from_owned_ptr(py, obj)
}
}
}
impl IntoPy<PyObject> for $rust_ty {
fn into_py(self, py: Python) -> PyObject {
self.to_object(py)
}
}
impl<'source> FromPyObject<'source> for $rust_ty {
fn extract(ob: &'source PyAny) -> PyResult<$rust_ty> {
let py = ob.py();
unsafe {
let num = ffi::PyNumber_Index(ob.as_ptr());
if num.is_null() {
return Err(PyErr::fetch(py));
}
let n_bits = ffi::_PyLong_NumBits(num);
let n_bytes = if n_bits < 0 {
return Err(PyErr::fetch(py));
} else if n_bits == 0 {
0
} else {
(n_bits as usize - 1 + $is_signed) / 8 + 1
};
let num: Py<PyLong> = Py::from_owned_ptr(py, num);
if n_bytes <= 128 {
let mut buffer = [0; 128];
extract(num.as_ref(py), &mut buffer[..n_bytes], $is_signed)?;
Ok($from_bytes(&buffer[..n_bytes]))
} else {
let mut buffer = vec![0; n_bytes];
extract(num.as_ref(py), &mut buffer, $is_signed)?;
Ok($from_bytes(&buffer))
}
}
}
}
};
}
bigint_conversion!(BigUint, 0, BigUint::to_bytes_le, BigUint::from_bytes_le);
bigint_conversion!(
BigInt,
1,
BigInt::to_signed_bytes_le,
BigInt::from_signed_bytes_le
);

#[cfg(test)]
mod test {
use super::*;
use crate::types::{PyDict, PyModule};
use indoc::indoc;

fn python_fib(py: Python) -> &PyModule {
let fib_code = indoc!(
r#"
def fib(n):
f0, f1 = 0, 1
for _ in range(n):
f0, f1 = f1, f0 + f1
return f0

def fib_neg(n):
return -fib(n)
"#
);
PyModule::from_code(py, fib_code, "fib.py", "fib").unwrap()
}

fn rust_fib<T>(n: usize) -> T
where
T: From<u16>,
for<'a> &'a T: std::ops::Add<Output = T>,
{
let mut f0: T = T::from(0);
let mut f1: T = T::from(1);
for _ in 0..n {
let f2 = &f0 + &f1;
f0 = std::mem::replace(&mut f1, f2);
}
f0
}

#[test]
fn convert_biguint() {
let gil = Python::acquire_gil();
let py = gil.python();
let rs_result: BigUint = rust_fib(400);
let fib = python_fib(py);
let locals = PyDict::new(py);
locals.set_item("rs_result", &rs_result).unwrap();
locals.set_item("fib", fib).unwrap();
// Checks if Rust BigUint -> Python Long conversion is correct
py.run("assert fib.fib(400) == rs_result", None, Some(locals))
.unwrap();
// Checks if Python Long -> Rust BigUint conversion is correct if N is small
let py_result: BigUint =
FromPyObject::extract(fib.getattr("fib").unwrap().call1((400,)).unwrap()).unwrap();
assert_eq!(rs_result, py_result);
// Checks if Python Long -> Rust BigUint conversion is correct if N is large
let rs_result: BigUint = rust_fib(2000);
let py_result: BigUint =
FromPyObject::extract(fib.getattr("fib").unwrap().call1((2000,)).unwrap()).unwrap();
assert_eq!(rs_result, py_result);
}

#[test]
fn convert_bigint() {
let gil = Python::acquire_gil();
let py = gil.python();
let rs_result = rust_fib::<BigInt>(400) * -1;
let fib = python_fib(py);
let locals = PyDict::new(py);
locals.set_item("rs_result", &rs_result).unwrap();
locals.set_item("fib", fib).unwrap();
// Checks if Rust BigInt -> Python Long conversion is correct
py.run("assert fib.fib_neg(400) == rs_result", None, Some(locals))
.unwrap();
// Checks if Python Long -> Rust BigInt conversion is correct if N is small
let py_result: BigInt =
FromPyObject::extract(fib.getattr("fib_neg").unwrap().call1((400,)).unwrap()).unwrap();
assert_eq!(rs_result, py_result);
// Checks if Python Long -> Rust BigInt conversion is correct if N is large
let rs_result = rust_fib::<BigInt>(2000) * -1;
let py_result: BigInt =
FromPyObject::extract(fib.getattr("fib_neg").unwrap().call1((2000,)).unwrap()).unwrap();
assert_eq!(rs_result, py_result);
}

fn python_index_class(py: Python) -> &PyModule {
let index_code = indoc!(
r#"
class C:
def __init__(self, x):
self.x = x
def __index__(self):
return self.x
"#
);
PyModule::from_code(py, index_code, "index.py", "index").unwrap()
}

#[test]
fn convert_index_class() {
let gil = Python::acquire_gil();
let py = gil.python();
let index = python_index_class(py);
let locals = PyDict::new(py);
locals.set_item("index", index).unwrap();
let ob = py.eval("index.C(10)", None, Some(locals)).unwrap();
let _: BigInt = FromPyObject::extract(ob).unwrap();
}

#[test]
fn handle_zero() {
let gil = Python::acquire_gil();
let py = gil.python();
let fib = python_fib(py);
let zero: BigInt =
FromPyObject::extract(fib.getattr("fib").unwrap().call1((0,)).unwrap()).unwrap();
assert_eq!(zero, BigInt::from(0));
}

/// `OverflowError` on converting Python int to BigInt, see issue #629
#[test]
fn check_overflow() {
let gil = Python::acquire_gil();
let py = gil.python();
macro_rules! test {
($T:ty, $value:expr, $py:expr) => {
let value = $value;
println!("{}: {}", stringify!($T), value);
let python_value = value.clone().to_object(py);
let roundtrip_value = python_value.extract::<$T>(py).unwrap();
assert_eq!(value, roundtrip_value);
};
}
for i in 0..=256usize {
// test a lot of values to help catch other bugs too
test!(BigInt, BigInt::from(i), py);
test!(BigUint, BigUint::from(i), py);
test!(BigInt, -BigInt::from(i), py);
test!(BigInt, BigInt::from(1) << i, py);
test!(BigUint, BigUint::from(1u32) << i, py);
test!(BigInt, -BigInt::from(1) << i, py);
test!(BigInt, (BigInt::from(1) << i) + 1u32, py);
test!(BigUint, (BigUint::from(1u32) << i) + 1u32, py);
test!(BigInt, (-BigInt::from(1) << i) + 1u32, py);
test!(BigInt, (BigInt::from(1) << i) - 1u32, py);
test!(BigUint, (BigUint::from(1u32) << i) - 1u32, py);
test!(BigInt, (-BigInt::from(1) << i) - 1u32, py);
}
}
}
Loading