-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Updating numpy.i (cvxcore) swig interface #2392
Conversation
Transurgeon seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
I think that's my preference tbh... I don't think we're in a huge rush to get this merged and I feel like we'll be far from the only package that is broken right after 2.0 drops. |
@PTNobel @SteveDiamond @rileyjmurray This could also maybe allow the cvxpy-feedstock to rebuild without any problems. What are your thoughts? |
That sounds like a great first step. I think I agree with h-vetenari in another thread that we should release 1.5.2 with NumPy 2.0 support before 2.0 comes out. That'll mean some changes to the GitHub action runners |
Ok, I just reverted the changes to |
I want to make sure that the cvxpy-base build is working before merging... Rerunning CI, since the error seemed spurious |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not a huge fan of downgrading our swig version. I had thought we had updated to fix a bug; William can you update the swig on your computer and rerun this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried rerunning the command, but to no avail. I also couldn't find a way to upgrade the swig version (sorry! I am very unfamiliar with swig).
I decided to just revert the changes to the file since it seems like it was already generated using the desired swig 4.1.1.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok I just managed to install swig 4.2.1 (the latest). And regenerated the cvxcore wrap file. Everything seems to work now :).
Upgrading to Swig 4.1.1 was quite intentional. I don't think we should backout #2273 |
Sorry, I wasn't aware of that PR. I just tried rebuilding cvxpy with numpy 2.0, but it didn't work. So we do need to rerun the given command with a newer version of swig. Anaconda only provides swig 4.0.2, see here: https://anaconda.org/anaconda/swig. Same thing for homebrew. (This means we should probably update the instructions in the cvxcore README as well). I found the releases on the swig GitHub, see here: https://github.com/swig/swig/tags. Would we potentially want to update to the latest version which is 4.2.1? |
@PTNobel could you have another look at this PR please? I managed to install the newest swig version and regenerated the cvxcore wrap file (it is now version 4.2.1). |
Looks good to me |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's see if this works with the pre-release CI!
Description
This PR updates the cvxpy build to allow compatible wheels with NumPy 2.0.
The changes come in the following stages:
numpy.i
file in cvxcore which is used for the swig interface. (As pointed out by Riley in the corresponding issue linked below). The new file is taken from here and was last updated 2 years ago.So I believe the updates to cvxcore are necessary for wheels compatibility with NumPy 2.0.
Any feedback/questions are welcome!
P.S: I am quite new to the python build ecosystem so I may have missed some important steps.
Issue link (if applicable): #2389
Type of change
Contribution checklist