-
Notifications
You must be signed in to change notification settings - Fork 38
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
Build wheels #31
Build wheels #31
Conversation
86862df
to
3079e93
Compare
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 project maintainer. These are just my two cents :)
@@ -18,7 +18,7 @@ References | |||
``` | |||
|
|||
### Installation | |||
`pip install quadprog` | |||
`pip install quadprog-wheel` |
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.
This should go back to quadprog
for merging I guess.
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.
It looks like I used the same branch for the PR and for creating the quadprog-wheel
distribution!
I'll fix that...
See [docs/DEVELOP.md](docs/DEVELOP.md). |
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.
Hmm, what changed here 🤔
|
||
setup( | ||
name="quadprog", | ||
name="quadprog-wheel", |
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.
Same comment here for merge-ability.
|
||
jobs: | ||
build: |
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.
Why remove the non-wheels build?
Perhaps it becomes redundant afterwards, but reducing the number of changes in this PR would also make it easier for project maintainers review it.
name: sdist | ||
path: dist/quadprog*.tar.gz | ||
|
||
test: |
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.
Same comment for test
.
Eliminating redundancies can also be done in a subsequent PR :)
version="0.1.11", | ||
description="Quadratic Programming Solver", | ||
description="Quadratic Programming Solver with wheel packages", |
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.
This is the description for https://pypi.org/project/quadprog-wheel/, can also be reverted to the original for merging.
ext_modules=extensions, | ||
install_requires=["numpy"] | ||
ext_modules=ext_modules, | ||
python_requires=">=3.6, <3.11", |
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.
Is the new python_requires
required for the PR to work?
Although there is no classifier for it I believe quadprog
can still be installed with Python 2.
Many thanks @AntoineD for this and @stephane-caron for your comments. I have taken the I have also built and published wheels for v0.1.11, and added instructions to build and publish wheels for future releases. |
This PR adds building the package wheels on linux, macos and windows for python 3.6, 3.7, 3.8 and 3.9.
This is based on cibuildwheel, each wheel is tested with the unit tests.
A tox configuration file is added to easily run the test by just running
tox
in a console.