search

louisabraham / pydivsufsort

Python package for string algorithms ➰
MIT License
38 stars 4 forks source link

Upload Built Wheels to PyPI #37

Closed seanlaw closed 2 years ago

seanlaw commented 2 years ago

Currently, we have a Github Actions Workflow building all of the appropriate wheels and a source distribution (sdist). However, they still need to be uploaded to PyPI via Twine (don't forget to bump the version in setup.py!!).

seanlaw commented 2 years ago

@louisabraham I've done all that I can with the Github Actions migration. Would you mind handling this part? Feel free to ask any clarifying questions!

louisabraham commented 2 years ago

Sure, will do.

seanlaw commented 2 years ago

@louisabraham Currently, the Github Actions will trigger every time new code is committed to the repo OR if there is a PR. You'll probably want to consider possibly creating different Github Actions "workflows" for PRs versus a release.

louisabraham commented 2 years ago

Ok I think I did it. I created lighter workflow for most purposes. build-and-upload is only launched when a commit modifying setup.py (where the version is specified) is pushed to master. We could even read the version from a separate file to isolate version changes even more.

seanlaw commented 2 years ago

Ok I think I did it. I created lighter workflow for most purposes. build-and-upload is only launched when a commit modifying setup.py (where the version is specified) is pushed to master. We could even read the version from a separate file to isolate version changes even more.

Awesome! Thanks for working on it. From what I've gleaned from other projects, it is common to trigger Github Actions workflows like build-and-upload based on the commit message OR if the commit is tagged:

https://stackoverflow.com/questions/61891328/trigger-github-action-only-on-new-tags

So, build-and-upload doesn't trigger every time and only if you manually tag a commit with, say, "Release version 0.06" (and then you'd match for "Release version " or something like that)

louisabraham commented 2 years ago

I prefer my solution of watching setup.py. Also, there is no risk to overwrite a version.

One run is not ending: https://github.com/louisabraham/pydivsufsort/runs/4499216537

louisabraham commented 2 years ago

I also fixed the code coverage: https://app.codecov.io/gh/louisabraham/pydivsufsort

seanlaw commented 2 years ago

I prefer my solution of watching setup.py

I see. So you are allowing the workflow to trigger only on push and when setup.py is modified (I guess only you and I have permission to do this). And then you have the test workflow for PRs. Cool!

louisabraham commented 2 years ago

It also has to run on the master branch, and only we two can push to master. I relaunched the job that didn't run to completion, this time I'll not stop it (github might after 6 hours).

It doesn't seem stuck, just very slow: https://github.com/louisabraham/pydivsufsort/runs/4499663161?check_suite_focus=true

I think it's because of our numpy requirement as there are no wheels uploaded for that old version and we must recompile numpy ourselves for each version.

seanlaw commented 2 years ago

I think it's because of our numpy requirement as there are no wheels uploaded for that old version and we must recompile numpy ourselves for each version.

But what actually changed from what my PR to now? All of my wheels were built successfully.

louisabraham commented 2 years ago

Oh, good point. Nothing changed. Your PR built all the wheels in 2h30 so I should expect the same timing: https://github.com/louisabraham/pydivsufsort/runs/4168448448?check_suite_focus=true

I think my diagnosis is right. Search the logs for "Building wheels for collected packages: numpy". It's really this step that makes one build go from 1 minute to 15 minutes.

On some platforms we have to build numpy twice: once for building (oldest-supported-numpy) and one for testing (numpy). I think this could be improved by asking the numpy folks to push wheels.

louisabraham commented 2 years ago

see https://github.com/numpy/numpy/issues/20573

louisabraham commented 2 years ago

it finished! What a nice sight :)

image
seanlaw commented 2 years ago

That's great! Thanks again for pushing it through the final meter!

I think what we've done will make it easier to maintain and for people to contribute to

louisabraham commented 2 years ago

I excluded musl from the builds since people need to build numpy anyway for the installation. Also, upgrading pip allows it to find wheels for cp310-musllinux_x86_64. This reduced the build time from 2h30 to 15 min.

I kept cp310-manylinux_i686 even though we have to build oldest-supported-numpy so that people can install pydivsufsort without compilation since there are wheels for numpy. It takes about 5 minutes.

louisabraham commented 2 years ago

Actually I was wrong about both points. cp310-manylinux_i686 is just using the cached wheels because oldest-supported and numpy currently match. Anyway, it's not a big problem to keep it for the moment.