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

Add deployment/testing with Python 3.10 #1593

Merged
merged 20 commits into from
Dec 27, 2021
Merged

Add deployment/testing with Python 3.10 #1593

merged 20 commits into from
Dec 27, 2021

Conversation

SteveDiamond
Copy link
Collaborator

No description provided.

@SteveDiamond
Copy link
Collaborator Author

SteveDiamond commented Dec 24, 2021

@phschiele @akshayka I'm getting pretty close to getting 3.10 working. I'm getting a weird bug I can't reproduce locally involving diffcp: https://github.com/cvxpy/cvxpy/runs/4628602909?check_suite_focus=true#step:6:4333
I'll think about it more but if anything comes to mind let me know.

Edit: Looks like SCS is failing to solve the problem...

@phschiele
Copy link
Collaborator

@SteveDiamond I think I found the issue:
Installing diffcp under Python 3.10 installs 1.0.16 instead of 1.0.18, which does not have support for SCS 3.0 yet.
The reason is that on PyPI there are neither wheels for 3.10, nor the source.
I'll check why the source was not deployed with the last tag. After this is fixed, the tests should pass here, and you could even undo the changes made to diffcp_conif.py, as this is handled within diffcp.

@phschiele
Copy link
Collaborator

@SteveDiamond I found two more occurrences where the Python versions are listed explicitly: pyproject.toml and sonar-project.properties.

SteveDiamond and others added 2 commits December 25, 2021 10:29
Co-authored-by: phschiele <44360364+phschiele@users.noreply.github.com>
@SteveDiamond
Copy link
Collaborator Author

SteveDiamond commented Dec 25, 2021

@phschiele thanks so much for figuring it out! I'm also having some problems building wheels for 3.10. Not totally sure what's going on there: https://github.com/cvxgrp/diffcp/runs/4632902674?check_suite_focus=true#step:7:53

Edit: I tried fixing this by upping the cibuildwheel version, but then windows builds failed :(

@SteveDiamond
Copy link
Collaborator Author

Integrating a version of #1559

@SteveDiamond
Copy link
Collaborator Author

SteveDiamond commented Dec 26, 2021

@phschiele I think this is good to go. Could you review it when you have time?

Edit: Nvm, it's failing to build wheels on ubuntu.

Copy link
Collaborator

@phschiele phschiele left a comment

Choose a reason for hiding this comment

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

@SteveDiamond This looks good 🚀
I hade one small remark about the diffcp interface above.

A second detail is that some of the environment variables like the PyPI settings are now unused in the build workflow and could be removed there.

cvxpy/reductions/solvers/conic_solvers/diffcp_conif.py Outdated Show resolved Hide resolved
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

100.0% 100.0% Coverage
0.0% 0.0% Duplication

- name: Build wheels
if: ${{env.DEPLOY == 'True' && env.USE_OPENMP != 'True'}}
if: ${{'True' == 'True' && env.USE_OPENMP != 'True'}}
Copy link
Collaborator

Choose a reason for hiding this comment

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

@SteveDiamond Just a reminder to undo these shortcuts after everything is confirmed working.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks! I'm going to let it build the wheels once on master and then undo them.

@SteveDiamond SteveDiamond merged commit a95a0a2 into master Dec 27, 2021
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.

2 participants