Skip to content

Port to PyO3 0.27 - #1941

Closed
xanderlent wants to merge 1 commit into
huggingface:mainfrom
xanderlent:pyo3-0.27
Closed

xanderlent wants to merge 1 commit into
huggingface:mainfrom
xanderlent:pyo3-0.27

Conversation

@xanderlent

Copy link
Copy Markdown
Contributor

Reviving #1939 this time with issues hopefully addressed.

.extract::<Bound<PyList>>()?
.extract::<Bound<PyList>>()
.map_err(|casterr| {
Box::new(pyo3::exceptions::PyException::new_err(casterr.to_string()))

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is the line I'm least sure about. Please take a look.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I have no idea 馃ぃ

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For context, here's the error you get without this change:

   Compiling tokenizers-python v0.22.3-dev.0 (/Users/goldbaum/Documents/tokenizers/bindings/python)
error[E0277]: `?` couldn't convert the error: `NonNull<PyObject>: Send` is not satisfied
   --> src/utils/pretokenization.rs:59:44
    |
 59 |                 .extract::<Bound<PyList>>()?
    |                  --------------------------^ `NonNull<PyObject>` cannot be sent between threads safely
    |                  |
    |                  this can't be annotated with `?` because it has type `Result<_, CastError<'_, '_>>`
    |
    = help: within `CastError<'_, '_>`, the trait `Send` is not implemented for `NonNull<PyObject>`
    = note: the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait
note: required because it appears within the type `pyo3::Borrowed<'_, '_, pyo3::PyAny>`
   --> /Users/goldbaum/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/pyo3-0.27.2/src/instance.rs:931:12
    |
931 | pub struct Borrowed<'a, 'py, T>(NonNull<ffi::PyObject>, PhantomData<&'a Py<T>>, Python<'py>);
    |            ^^^^^^^^
note: required because it appears within the type `CastError<'_, '_>`
   --> /Users/goldbaum/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/pyo3-0.27.2/src/err/cast_error.rs:11:12
    |
 11 | pub struct CastError<'a, 'py> {
    |            ^^^^^^^^^
    = note: required for `Box<dyn StdError + Send + Sync>` to implement `From<CastError<'_, '_>>`

error[E0277]: `?` couldn't convert the error: `NonNull<PyObject>: Sync` is not satisfied
   --> src/utils/pretokenization.rs:59:44
    |
 59 |                 .extract::<Bound<PyList>>()?
    |                  --------------------------^ `NonNull<PyObject>` cannot be shared between threads safely
    |                  |
    |                  this can't be annotated with `?` because it has type `Result<_, CastError<'_, '_>>`
    |
    = help: within `CastError<'_, '_>`, the trait `Sync` is not implemented for `NonNull<PyObject>`
    = note: the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait
note: required because it appears within the type `pyo3::Borrowed<'_, '_, pyo3::PyAny>`
   --> /Users/goldbaum/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/pyo3-0.27.2/src/instance.rs:931:12
    |
931 | pub struct Borrowed<'a, 'py, T>(NonNull<ffi::PyObject>, PhantomData<&'a Py<T>>, Python<'py>);
    |            ^^^^^^^^
note: required because it appears within the type `CastError<'_, '_>`
   --> /Users/goldbaum/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/pyo3-0.27.2/src/err/cast_error.rs:11:12
    |
 11 | pub struct CastError<'a, 'py> {
    |            ^^^^^^^^^
    = note: required for `Box<dyn StdError + Send + Sync>` to implement `From<CastError<'_, '_>>`

error[E0277]: `?` couldn't convert the error: `*mut pyo3::Python<'static>: Send` is not satisfied
   --> src/utils/pretokenization.rs:59:44
    |
 59 |                 .extract::<Bound<PyList>>()?
    |                  --------------------------^ `*mut pyo3::Python<'static>` cannot be sent between threads safely
    |                  |
    |                  this can't be annotated with `?` because it has type `Result<_, CastError<'_, '_>>`
    |
    = help: within `CastError<'_, '_>`, the trait `Send` is not implemented for `*mut pyo3::Python<'static>`
    = note: the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait
note: required because it appears within the type `PhantomData<*mut pyo3::Python<'static>>`
   --> /Users/goldbaum/.rustup/toolchains/stable-aarch64-apple-darwin/lib/rustlib/src/rust/library/core/src/marker.rs:819:12
    |
819 | pub struct PhantomData<T: PointeeSized>;
    |            ^^^^^^^^^^^
note: required because it appears within the type `pyo3::marker::NotSend`
   --> /Users/goldbaum/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/pyo3-0.27.2/src/marker.rs:357:8
    |
357 | struct NotSend(PhantomData<*mut Python<'static>>);
    |        ^^^^^^^
note: required because it appears within the type `PhantomData<pyo3::marker::NotSend>`
   --> /Users/goldbaum/.rustup/toolchains/stable-aarch64-apple-darwin/lib/rustlib/src/rust/library/core/src/marker.rs:819:12
    |
819 | pub struct PhantomData<T: PointeeSized>;
    |            ^^^^^^^^^^^
note: required because it appears within the type `pyo3::Python<'_>`
   --> /Users/goldbaum/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/pyo3-0.27.2/src/marker.rs:353:12
    |
353 | pub struct Python<'py>(PhantomData<&'py AttachGuard>, PhantomData<NotSend>);
    |            ^^^^^^
note: required because it appears within the type `pyo3::Borrowed<'_, '_, pyo3::PyAny>`
   --> /Users/goldbaum/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/pyo3-0.27.2/src/instance.rs:931:12
    |
931 | pub struct Borrowed<'a, 'py, T>(NonNull<ffi::PyObject>, PhantomData<&'a Py<T>>, Python<'py>);
    |            ^^^^^^^^
note: required because it appears within the type `CastError<'_, '_>`
   --> /Users/goldbaum/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/pyo3-0.27.2/src/err/cast_error.rs:11:12
    |
 11 | pub struct CastError<'a, 'py> {
    |            ^^^^^^^^^
    = note: required for `Box<dyn StdError + Send + Sync>` to implement `From<CastError<'_, '_>>`

error[E0277]: `?` couldn't convert the error: `*mut pyo3::Python<'static>: Sync` is not satisfied
   --> src/utils/pretokenization.rs:59:44
    |
 59 |                 .extract::<Bound<PyList>>()?
    |                  --------------------------^ `*mut pyo3::Python<'static>` cannot be shared between threads safely
    |                  |
    |                  this can't be annotated with `?` because it has type `Result<_, CastError<'_, '_>>`
    |
    = help: within `CastError<'_, '_>`, the trait `Sync` is not implemented for `*mut pyo3::Python<'static>`
    = note: the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait
note: required because it appears within the type `PhantomData<*mut pyo3::Python<'static>>`
   --> /Users/goldbaum/.rustup/toolchains/stable-aarch64-apple-darwin/lib/rustlib/src/rust/library/core/src/marker.rs:819:12
    |
819 | pub struct PhantomData<T: PointeeSized>;
    |            ^^^^^^^^^^^
note: required because it appears within the type `pyo3::marker::NotSend`
   --> /Users/goldbaum/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/pyo3-0.27.2/src/marker.rs:357:8
    |
357 | struct NotSend(PhantomData<*mut Python<'static>>);
    |        ^^^^^^^
note: required because it appears within the type `PhantomData<pyo3::marker::NotSend>`
   --> /Users/goldbaum/.rustup/toolchains/stable-aarch64-apple-darwin/lib/rustlib/src/rust/library/core/src/marker.rs:819:12
    |
819 | pub struct PhantomData<T: PointeeSized>;
    |            ^^^^^^^^^^^
note: required because it appears within the type `pyo3::Python<'_>`
   --> /Users/goldbaum/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/pyo3-0.27.2/src/marker.rs:353:12
    |
353 | pub struct Python<'py>(PhantomData<&'py AttachGuard>, PhantomData<NotSend>);
    |            ^^^^^^
note: required because it appears within the type `pyo3::Borrowed<'_, '_, pyo3::PyAny>`
   --> /Users/goldbaum/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/pyo3-0.27.2/src/instance.rs:931:12
    |
931 | pub struct Borrowed<'a, 'py, T>(NonNull<ffi::PyObject>, PhantomData<&'a Py<T>>, Python<'py>);
    |            ^^^^^^^^
note: required because it appears within the type `CastError<'_, '_>`
   --> /Users/goldbaum/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/pyo3-0.27.2/src/err/cast_error.rs:11:12
    |
 11 | pub struct CastError<'a, 'py> {
    |            ^^^^^^^^^
    = note: required for `Box<dyn StdError + Send + Sync>` to implement `From<CastError<'_, '_>>`

@davidhewitt since you were responsible for CastError in PyO3 0.27 - do you happen to know how to handle this without converting errors to strings like this PR does?

I'm a little lost in the tokenizers codebase unfortunately because for whatever reason my emacs LSP setup refuses to work for mysterious reasons, so I'm handicapped without types looking at the source code...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think PyErr::from(cast_error) should be enough. Also works as .map_err(PyErr::from)

@HuggingFaceDocBuilderDev

Copy link
Copy Markdown

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@xanderlent

Copy link
Copy Markdown
Contributor Author

@davidhewitt

Copy link
Copy Markdown
Contributor

Looks like this was done in #1928

@ArthurZucker

Copy link
Copy Markdown
Collaborator

yep closing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants