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 PyO3 to 0.22 #1293

Merged
merged 12 commits into from
Oct 14, 2024
Merged

Update PyO3 to 0.22 #1293

merged 12 commits into from
Oct 14, 2024

Conversation

IvanIsCoding
Copy link
Collaborator

@IvanIsCoding IvanIsCoding commented Oct 10, 2024

Closes #1250

This depends on:

  • rust-numpy releasing 0.22, which will happen soon. For the time being, we build from their Git main branch
  • py-clone feature is enabled, if it is disabled too many things fail
  • The update broke iterators.rs, I haven't had the time to fix so for this draft I just deleted support for slicing. Of course the final version will need to reintroduce it Slicing is now fixed
  • We have 68 warnings! But hey it builds Warnings are fixed

I also profited from this PR to update ndarray and ndarray-stats.

For py-clone I think we should handle it in a future PR

@IvanIsCoding IvanIsCoding changed the title [WIP] Update PyO3 to 0.22 Update PyO3 to 0.22 Oct 10, 2024
@IvanIsCoding
Copy link
Collaborator Author

IvanIsCoding commented Oct 10, 2024

Ok, this is ready just pending PyO3/rust-numpy#453. Although #1292 might break it because there are some deprecated methods.

Update: it worked fine after #1292 without issues

@coveralls
Copy link

coveralls commented Oct 11, 2024

Pull Request Test Coverage Report for Build 11331133743

Details

  • 10 of 10 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.002%) to 95.82%

Totals Coverage Status
Change from base Build 11284044701: 0.002%
Covered Lines: 18016
Relevant Lines: 18802

💛 - Coveralls

@IvanIsCoding
Copy link
Collaborator Author

Ok this is ready to go

@IvanIsCoding
Copy link
Collaborator Author

I think there is a bug in PyO3 0.22.4 released today (https://github.com/PyO3/pyo3/blob/release-0.22/CHANGELOG.md). Our code compiles with 0.22.3 but fails with 0.22.4. We should probably merge this as is an open an issue?

error[E0605]: non-primitive cast: `for<'py> unsafe fn(pyo3::Python<'py>, *mut pyo3::ffi::PyObject) -> Result<*mut pyo3::ffi::PyObject, pyo3::PyErr> {PyDiGraph::__pymethod_clear__}` as `unsafe extern "C" fn(*mut pyo3::ffi::PyObject) -> i32`
   --> src/digraph.rs:295:1
    |
295 | #[pymethods]
    | ^^^^^^^^^^^^ invalid cast
    |
    = note: this error originates in the attribute macro `pymethods` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0605]: non-primitive cast: `for<'py> unsafe fn(pyo3::Python<'py>, *mut pyo3::ffi::PyObject) -> Result<*mut pyo3::ffi::PyObject, pyo3::PyErr> {PyGraph::__pymethod_clear__}` as `unsafe extern "C" fn(*mut pyo3::ffi::PyObject) -> i32`
   --> src/graph.rs:191:1
    |
191 | #[pymethods]
    | ^^^^^^^^^^^^ invalid cast
    |
    = note: this error originates in the attribute macro `pymethods` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0592]: duplicate definitions with name `__pymethod_clear__`
   --> src/digraph.rs:295:1
    |
295 | #[pymethods]
    | ^^^^^^^^^^^^
    | |
    | duplicate definitions for `__pymethod_clear__`
    | other definition for `__pymethod_clear__`
    |
    = note: this error originates in the attribute macro `pymethods` (in Nightly builds, run with -Z macro-backtrace for more info)

@IvanIsCoding
Copy link
Collaborator Author

It is confirmed 0.22.4 has a regression: PyO3/pyo3#4616

We can bump to 0.22.5 once it is released.

Copy link
Member

@mtreinish mtreinish left a comment

Choose a reason for hiding this comment

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

Looks good, except for one small stray comment inline which I'll apply and re-approve for. Thanks for doing this. I agree we should update to 0.22.5 and remove py-clone in a follow-up PR.

src/iterators.rs Outdated Show resolved Hide resolved
@mtreinish mtreinish enabled auto-merge October 14, 2024 16:02
@mtreinish mtreinish added this pull request to the merge queue Oct 14, 2024
Merged via the queue into Qiskit:main with commit 6f1d2ec Oct 14, 2024
26 checks passed
@IvanIsCoding IvanIsCoding deleted the pyo3-022 branch October 14, 2024 17:31
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.

Update PyO3 to 0.22.x
3 participants