-
Notifications
You must be signed in to change notification settings - Fork 44
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 an action that builds wheels #1039
Conversation
This will eventually be folded into the release action.
], | ||
}, | ||
zip_safe=False, | ||
python_requires=">=3.7", |
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 only change here apart from formatting.
with: | ||
python-version: '3.8' | ||
python-version: '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.
Any reason why the sdist should be created with the latest possible version of Python? I would assume an older version was better (and Cython source would be compiled by the user of the sdist)
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.
No particular reason other than the assumption that most recent python would automatically get the most current packaging infrastructure. Cython shouldn't be an issue because I don't think we ship the generated .cpp files in the sdist any more.
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.
Looks like I was wrong about that:
Line 17 in 2230bb4
recursive-include kiva/agg *.py *.i *.cpp *.h *.c |
Probably should remove that - the isolated builds should install Cython correctly from the info in the sdist if needed when building.
Edit: Nope, that's all in the agg subdirectory.
And I also spot a stray gl reference in the manifest as well.
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.
LGTM
This adds a stand-alone build wheel action, and then adds it to the publish to PyPI action for releases. This builds wheels for Mac, Windows and Linux Intel architecture, as well as Linux ARM.
It doesn't handle Mac ARM or Universal, but it potentially could with some more work and testing.