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

Pin Numpy < 2 in requirements #1012

Merged
merged 1 commit into from
Oct 19, 2023
Merged

Conversation

mtreinish
Copy link
Member

We have compiled extensions that are built against the Numpy C API. The Numpy transition guide for developers of downstream packages[1] encourages us to put in this pin until we have wheels built against the newer version, which is not expected to be fully ABI compatible (but extensions built against Numpy 2 should work with older versions of Numpy).

Note that this won't prevent package managers from resolving older versions of rustworkx (which don't have the pin) along with Numpy 2, but there's not a vast amount we can do about that now.

We have compiled extensions that are built against the Numpy C API.
The Numpy transition guide for developers of downstream packages[1]
encourages us to put in this pin until we have wheels built against
the newer version, which is not expected to be fully ABI compatible
(but extensions built against Numpy 2 _should_ work with older versions
of Numpy).

Note that this won't prevent package managers from resolving _older_
versions of rustworkx (which don't have the pin) along with Numpy 2,
but there's not a vast amount we can do about that now.

Co-authored-by: Jake Lishman <jake.lishman@ibm.com>
@mtreinish mtreinish added this to the 0.14.0 milestone Oct 19, 2023
Copy link
Collaborator

@IvanIsCoding IvanIsCoding left a comment

Choose a reason for hiding this comment

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

LGTM

@IvanIsCoding IvanIsCoding added the automerge Queue a approved PR for merging label Oct 19, 2023
@coveralls
Copy link

Pull Request Test Coverage Report for Build 6580899183

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 96.546%

Totals Coverage Status
Change from base Build 6567628300: 0.0%
Covered Lines: 15680
Relevant Lines: 16241

💛 - Coveralls

@mergify mergify bot merged commit 5fa3fcd into Qiskit:main Oct 19, 2023
27 checks passed
@mtreinish mtreinish deleted the pin-numpy-less-than-2 branch October 19, 2023 22:45
mtreinish added a commit to mtreinish/retworkx that referenced this pull request Apr 24, 2024
In Qiskit#1012 we added an upper version cap to numpy to prevent it from
installing numpy 2.0 before we confirmed that rustworkx was compatible
with it. Now that numpy 2.0.0rc1 has been released we're able to confirm
that rustworkx works fine with numpy 2.0. This commit raises the upper
bound on the numpy version to < 3 to enable installing numpy 2.0 with
rustworkx.
IvanIsCoding added a commit that referenced this pull request Apr 27, 2024
* Relax numpy upper version cap

In #1012 we added an upper version cap to numpy to prevent it from
installing numpy 2.0 before we confirmed that rustworkx was compatible
with it. Now that numpy 2.0.0rc1 has been released we're able to confirm
that rustworkx works fine with numpy 2.0. This commit raises the upper
bound on the numpy version to < 3 to enable installing numpy 2.0 with
rustworkx.

* Handle new __array__ API in numpy 2.0

While we didn't have any test coverage for this looking at the numpy 2.0
migration guide one thing we'll have to handle is the new copy kwarg on
array:

https://numpy.org/devdocs/numpy_2_0_migration_guide.html#adapting-to-changes-in-the-copy-keyword

This commit updates the sole use of __array__ we have on custom sequence
return types so that if copy=False is passed in we raise a ValueError.
Additionally, the dtype handling is done directly in the rustworkx code
now to ensure we don't have any issues with numpy 2.0.

* Fix __array__ stubs

* Update src/iterators.rs

* Pin ruff to 0.4.1

---------

Co-authored-by: Ivan Carvalho <8753214+IvanIsCoding@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Queue a approved PR for merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants