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

Update to pyo3 0.12.0 #152

Merged
merged 9 commits into from
Sep 18, 2020
Merged

Update to pyo3 0.12.0 #152

merged 9 commits into from
Sep 18, 2020

Conversation

g-bauer
Copy link
Contributor

@g-bauer g-bauer commented Sep 4, 2020

I tried using the current pyo3 master branch but ran into problems.

This is the current error which I guess stems from the changes to PyDowncastError.

   --> src/array.rs:134:28
    |
134 |                 return Err(PyDowncastError.into());
    |                            ^^^^^^^^^^^^^^^-----
    |                            |
    |                            help: use the path separator to refer to an item: `PyDowncastError::into`

@g-bauer
Copy link
Contributor Author

g-bauer commented Sep 4, 2020

Compiles but tests are not working.

@davidhewitt
Copy link
Member

Thanks! AsPyRef is deleted on pyo3 master, you should be able to delete imports of it without needing to make any code changes. Hopefully that'll fix the tests

g-bauer and others added 3 commits September 7, 2020 14:06
In PyO3/pyo3#1115 the internal_utils module was removed from pyo3. There
is a note in the review that rust-numpy will need to migrate to using
with_gil() instead. This commit makes this change to enable running with
pyo3 0.12.0.
This commit finishes the migration of error.rs to use the new exception
API in 0.12.0. Most of the work was done on this, just a few little
pieces that needed to be updated to conform to the new api.
@mtreinish
Copy link
Contributor

@g-bauer I opened up https://github.com/g-bauer/rust-numpy/pull/1 that should finish the migration here. There were just a couple of other things that needed to be updated for 0.12.0 compat.

@g-bauer
Copy link
Contributor Author

g-bauer commented Sep 13, 2020

Great! Thanks @mtreinish

@mtreinish
Copy link
Contributor

@g-bauer cool, thanks for merging my PR. I think this is ready for review now, you probably can update the title and move this out of draft.

@g-bauer g-bauer changed the title Fixes to allow compilation with current pyo3 master Update to pyo3 0.12.0 Sep 14, 2020
@g-bauer g-bauer marked this pull request as ready for review September 14, 2020 12:29
Copy link
Member

@kngwyu kngwyu left a comment

Choose a reason for hiding this comment

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

@g-bauer
@mtreinish
Thank you very much for this.
I'll release the new version after fixing #151 and merging #155

src/array.rs Outdated
@@ -75,6 +75,7 @@ use crate::types::Element;
/// array![[8., 15.], [12., 23.]]
/// );
/// ```
#[derive(Debug)]
Copy link
Member

Choose a reason for hiding this comment

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

This adds PhatromData,PhantomData) to its debug print, which I don't like.
So could you please use pyobject_native_type_fmt!(PyArray<T, D>, T, D); instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@g-bauer
Copy link
Contributor Author

g-bauer commented Sep 16, 2020

With bugfixes coming to pyo3 0.12, maybe we can change the version in Cargo.toml to pyo3 = "~0.12"?

@kngwyu
Copy link
Member

kngwyu commented Sep 16, 2020

With bugfixes coming to pyo3 0.12, maybe we can change the version in Cargo.toml to pyo3 = "~0.12"?

I think just 0.12 would make sense.

@awaited-hare
Copy link
Contributor

pyo3 0.12 has released. Any plan to merge this?

@kngwyu kngwyu merged commit bacb424 into PyO3:master Sep 18, 2020
@g-bauer g-bauer deleted the pyo3_0.12_compatible branch September 18, 2020 07:51
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