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

Initial Migration to Github Actions #34

Merged

Conversation

seanlaw
Copy link
Collaborator

@seanlaw seanlaw commented Oct 31, 2021

Work-In-Progress. Do not merge!

This is not meant to be comprehensive and takes an initial stab at addressing #31

The Github Actions workflow that is currently being triggered can be viewed here.

TO DO

  • Generate sdist in addition to wheels (see TravisCI workflow)
  • Add explicit minimum NumPy build version

@seanlaw seanlaw changed the title [WIP - Do Not Merge] Separate Build Frontend and Backend [WIP - Do Not Merge] Migrate to Github Actions Nov 8, 2021
@seanlaw seanlaw changed the title [WIP - Do Not Merge] Migrate to Github Actions [WIP - Do Not Merge] Initial Migration to Github Actions Nov 8, 2021
@seanlaw
Copy link
Collaborator Author

seanlaw commented Nov 8, 2021

Instead of splitting the frontend/backend, we should just migrate to Github Actions first and have the structure in place to split things post-migration (if necessary).

@louisabraham I'm hoping that you can handle the twine uploading part once this PR gets merged. It'll require changing the last few lines of the github-actions.yml:

- name: Upload Builds
run: |
python -m pip install twine
if [ "$RUNNER_OS" == "Linux" ]; then
# Replace/remove echo
echo "python -m twine upload --skip-existing dist/*"
fi
# Replace/remove echo
echo "python -m twine upload --skip-existing wheelhouse/*.whl"
shell: bash

@Pierre-Sassoulas
Copy link

Pierre-Sassoulas commented Nov 8, 2021

Regarding the release here's an example of a workflow for it : https://github.com/PyCQA/pylint/blob/main/.github/workflows/release.yml

This will release the package when the release is created in github (tag created then release created by editing the tag description in interface). This can be copy pasted, only the value for:

          TWINE_REPOSITORY: pypi
          TWINE_USERNAME: __token__
          TWINE_PASSWORD: ${{ secrets.PYPI_API_TOKEN }}

needs to be added in the project settings.

@seanlaw
Copy link
Collaborator Author

seanlaw commented Nov 8, 2021

Regarding the release here's an example of a workflow for it : https://github.com/PyCQA/pylint/blob/main/.github/workflows/release.yml

This will release the package when the release is created in github (tag created then release created by editing the tag description in interface). This can be copy pasted, only the value for:

          TWINE_REPOSITORY: pypi
          TWINE_USERNAME: __token__
          TWINE_PASSWORD: ${{ secrets.PYPI_API_TOKEN }}

needs to be added in the project settings.

Thanks @Pierre-Sassoulas.

Frankly, @louisabraham I am not comfortable with handling this step as it will require upping the package version (even for test.pypi). I think this PR should be treated as a proof-of-concept that wheels/sdist and can be built successfully using Github Actions, which I believe is the case (save for correctly choosing the minimum NumPy version support by all platforms). I'll need your help to take it the last mile/meter.

@Pierre-Sassoulas
Copy link

I am not comfortable with handling this step as it will require upping the package version.

You can test the release by creating an alpha version with a tag: 0.0.5-a0 it won't be recoverable with pip unless users really asks for it with pip install pydivsufsort==0.0.5-a0.

@seanlaw
Copy link
Collaborator Author

seanlaw commented Nov 8, 2021

You can test the release by creating an alpha version with a tag: 0.0.5-a0 it won't be recoverable with pip unless users really asks for it with pip install pydivsufsort==0.0.5-a0.

My comfort level has only ever-so-slightly increased with this knowledge. 😄

@Pierre-Sassoulas Do you think this is something that you can help us with after this PR is merged? I am limited on time and, for this PR, I want to focus on the last major item that will allow us to have stable builds and at least put us in a better position to make progress:

  • Choose minimum NumPy version available to all platforms/Python versions that we want to support

@Pierre-Sassoulas
Copy link

I don't have enough time to do it myself but I can help you with it if you have questions. Also I have an example of such a migration: landscapeio/prospector#427

Pylint also has a coverage job too if you want some inspiration: https://github.com/PyCQA/pylint/blob/main/.github/workflows/ci.yaml#L228

Regarding the minimum numpy version it depends on the deprecation of numpy and of the API you're using I don't know much about numpy nor what you're using. Determining the absolute minimum number might take some time. The lazy bet would be to support what users asks for and do it and test that it works when they do ? (Right now it seems it's numpy 1.19.5 for tensorflow 2.6.0)

@seanlaw
Copy link
Collaborator Author

seanlaw commented Nov 8, 2021

I went through all of the cibuildwheel logs and recorded the minimum NumPy versions that were recognized:

"cp36-manylinux_x86_64 wheel","1.11.3, 1.12.0, 1.12.1, 1.13.0rc1, 1.13.0rc2, 1.13.0, 1.13.1, 1.13.3, 1.14.0rc1, 1.14.0, 1.14.1, 1.14.2, 1.14.3, 1.14.4, 1.14.5, 1.14.6, 1.15.0rc1, 1.15.0rc2, 1.15.0, 1.15.1, 1.15.2, 1.15.3, 1.15.4, 1.16.0rc1, 1.16.0rc2, 1.16.0, 1.16.1, 1.16.2, 1.16.3, 1.16.4, 1.16.5, 1.16.6, 1.17.0rc1, 1.17.0rc2, 1.17.0, 1.17.1, 1.17.2, 1.17.3, 1.17.4, 1.17.5, 1.18.0rc1, 1.18.0, 1.18.1, 1.18.2, 1.18.3, 1.18.4, 1.18.5, 1.19.0rc1, 1.19.0rc2, 1.19.0, 1.19.1, 1.19.2, 1.19.3, 1.19.4, 1.19.5"
"cp37-manylinux_x86_64 wheel","1.14.5, 1.14.6, 1.15.0rc2, 1.15.0, 1.15.1, 1.15.2, 1.15.3, 1.15.4, 1.16.0rc1, 1.16.0rc2, 1.16.0, 1.16.1, 1.16.2, 1.16.3, 1.16.4, 1.16.5, 1.16.6, 1.17.0rc1, 1.17.0rc2, 1.17.0, 1.17.1, 1.17.2, 1.17.3, 1.17.4, 1.17.5, 1.18.0rc1, 1.18.0, 1.18.1, 1.18.2, 1.18.3, 1.18.4, 1.18.5, 1.19.0rc1, 1.19.0rc2, 1.19.0, 1.19.1, 1.19.2, 1.19.3, 1.19.4, 1.19.5, 1.20.0rc1, 1.20.0rc2, 1.20.0, 1.20.1, 1.20.2, 1.20.3, 1.21.0rc1, 1.21.0rc2, 1.21.0, 1.21.1, 1.21.2, 1.21.3, 1.21.4"
"cp38-manylinux_x86_64 wheel","1.17.3, 1.17.4, 1.17.5, 1.18.0rc1, 1.18.0, 1.18.1, 1.18.2, 1.18.3, 1.18.4, 1.18.5, 1.19.0rc1, 1.19.0rc2, 1.19.0, 1.19.1, 1.19.2, 1.19.3, 1.19.4, 1.19.5, 1.20.0rc1, 1.20.0rc2, 1.20.0, 1.20.1, 1.20.2, 1.20.3, 1.21.0rc1, 1.21.0rc2, 1.21.0, 1.21.1, 1.21.2, 1.21.3, 1.21.4"
"cp39-manylinux_x86_64 wheel","1.19.3, 1.19.4, 1.19.5, 1.20.0rc1, 1.20.0rc2, 1.20.0, 1.20.1, 1.20.2, 1.20.3, 1.21.0rc1, 1.21.0rc2, 1.21.0, 1.21.1, 1.21.2, 1.21.3, 1.21.4"
"cp310-manylinux_x86_64 wheel",""
"cp36-manylinux_i686 wheel","1.11.3, 1.12.0, 1.12.1, 1.13.0rc1, 1.13.0rc2, 1.13.0, 1.13.1, 1.13.3, 1.14.0rc1, 1.14.0, 1.14.1, 1.14.2, 1.14.3, 1.14.4, 1.14.5, 1.14.6, 1.15.0rc1, 1.15.0rc2, 1.15.0, 1.15.1, 1.15.2, 1.15.3, 1.15.4, 1.16.0rc1, 1.16.0rc2, 1.16.0, 1.16.1, 1.16.2, 1.16.3, 1.16.4, 1.16.5, 1.16.6, 1.17.0rc1, 1.17.0rc2, 1.17.0, 1.17.1, 1.17.2, 1.17.3, 1.17.4, 1.17.5, 1.18.0rc1, 1.18.0, 1.18.1, 1.18.2, 1.18.3, 1.18.4, 1.18.5, 1.19.0rc1, 1.19.0rc2, 1.19.0, 1.19.1, 1.19.2, 1.19.3, 1.19.4, 1.19.5"
"cp37-manylinux_i686 wheel","1.14.5, 1.14.6, 1.15.0rc2, 1.15.0, 1.15.1, 1.15.2, 1.15.3, 1.15.4, 1.16.0rc1, 1.16.0rc2, 1.16.0, 1.16.1, 1.16.2, 1.16.3, 1.16.4, 1.16.5, 1.16.6, 1.17.0rc1, 1.17.0rc2, 1.17.0, 1.17.1, 1.17.2, 1.17.3, 1.17.4, 1.17.5, 1.18.0rc1, 1.18.0, 1.18.1, 1.18.2, 1.18.3, 1.18.4, 1.18.5, 1.19.0rc1, 1.19.0rc2, 1.19.0, 1.19.1, 1.19.2, 1.19.3, 1.19.4, 1.19.5, 1.20.0rc1, 1.20.0rc2, 1.20.0, 1.20.1, 1.20.2, 1.20.3, 1.21.0rc1, 1.21.0rc2, 1.21.0, 1.21.1, 1.21.2, 1.21.3, 1.21.4"
"cp38-manylinux_i686 wheel","1.17.3, 1.17.4, 1.17.5, 1.18.0rc1, 1.18.0, 1.18.1, 1.18.2, 1.18.3, 1.18.4, 1.18.5, 1.19.0rc1, 1.19.0rc2, 1.19.0, 1.19.1, 1.19.2, 1.19.3, 1.19.4, 1.19.5, 1.20.0rc1, 1.20.0rc2, 1.20.0, 1.20.1, 1.20.2, 1.20.3, 1.21.0rc1, 1.21.0rc2, 1.21.0, 1.21.1, 1.21.2, 1.21.3, 1.21.4"
"cp39-manylinux_i686 wheel","1.19.3, 1.19.4, 1.19.5, 1.20.0rc1, 1.20.0rc2, 1.20.0, 1.20.1, 1.20.2, 1.20.3, 1.21.0rc1, 1.21.0rc2, 1.21.0, 1.21.1, 1.21.2, 1.21.3, 1.21.4"
"cp310-manylinux_i686 wheel",""
"cp36-musllinux_x86_64 wheel",""
"cp37-musllinux_x86_64 wheel",""
"cp38-musllinux_x86_64 wheel",""
"cp39-musllinux_x86_64 wheel",""
"cp310-musllinux_x86_64 wheel",""
"cp36-musllinux_i686 wheel",""
"cp37-musllinux_i686 wheel",""
"cp38-musllinux_i686 wheel",""
"cp39-musllinux_i686 wheel",""
"cp310-musllinux_i686 wheel",""
"cp36-macosx_x86_64 wheel","1.11.3, 1.12.0, 1.12.1, 1.13.0rc1, 1.13.0rc2, 1.13.0, 1.13.1, 1.13.3, 1.14.0rc1, 1.14.0, 1.14.1, 1.14.2, 1.14.3, 1.14.4, 1.14.5, 1.14.6, 1.15.0rc1, 1.15.0rc2, 1.15.0, 1.15.1, 1.15.2, 1.15.3, 1.15.4, 1.16.0rc1, 1.16.0rc2, 1.16.0, 1.16.1, 1.16.2, 1.16.3, 1.16.4, 1.16.5, 1.16.6, 1.17.0rc1, 1.17.0rc2, 1.17.0, 1.17.1, 1.17.2, 1.17.3, 1.17.4, 1.17.5, 1.18.0rc1, 1.18.0, 1.18.1, 1.18.2, 1.18.3, 1.18.4, 1.18.5, 1.19.0rc1, 1.19.0rc2, 1.19.0, 1.19.1, 1.19.2, 1.19.3, 1.19.4, 1.19.5"
"cp37-macosx_x86_64 wheel","1.14.5, 1.14.6, 1.15.0rc2, 1.15.0, 1.15.1, 1.15.2, 1.15.3, 1.15.4, 1.16.0rc1, 1.16.0rc2, 1.16.0, 1.16.1, 1.16.2, 1.16.3, 1.16.4, 1.16.5, 1.16.6, 1.17.0rc1, 1.17.0rc2, 1.17.0, 1.17.1, 1.17.2, 1.17.3, 1.17.4, 1.17.5, 1.18.0rc1, 1.18.0, 1.18.1, 1.18.2, 1.18.3, 1.18.4, 1.18.5, 1.19.0rc1, 1.19.0rc2, 1.19.0, 1.19.1, 1.19.2, 1.19.3, 1.19.4, 1.19.5, 1.20.0rc1, 1.20.0rc2, 1.20.0, 1.20.1, 1.20.2, 1.20.3, 1.21.0rc1, 1.21.0rc2, 1.21.0, 1.21.1, 1.21.2, 1.21.3, 1.21.4"
"cp38-macosx_x86_64 wheel","1.17.3, 1.17.4, 1.17.5, 1.18.0rc1, 1.18.0, 1.18.1, 1.18.2, 1.18.3, 1.18.4, 1.18.5, 1.19.0rc1, 1.19.0rc2, 1.19.0, 1.19.1, 1.19.2, 1.19.3, 1.19.4, 1.19.5, 1.20.0rc1, 1.20.0rc2, 1.20.0, 1.20.1, 1.20.2, 1.20.3, 1.21.0rc1, 1.21.0rc2, 1.21.0, 1.21.1, 1.21.2, 1.21.3, 1.21.4"
"cp39-macosx_x86_64 wheel","1.19.3, 1.19.4, 1.19.5, 1.20.0rc1, 1.20.0rc2, 1.20.0, 1.20.1, 1.20.2, 1.20.3, 1.21.0rc1, 1.21.0rc2, 1.21.0, 1.21.1, 1.21.2, 1.21.3, 1.21.4"
"cp310-macosx_x86_64 wheel","1.21.3, 1.21.4"
"cp36-win32 wheel","1.12.0, 1.12.1, 1.13.0rc1, 1.13.0rc2, 1.13.0, 1.13.1, 1.13.3, 1.14.0rc1, 1.14.0, 1.14.1, 1.14.2, 1.14.3, 1.14.4, 1.14.5, 1.14.6, 1.15.0rc1, 1.15.0rc2, 1.15.0, 1.15.1, 1.15.2, 1.15.3, 1.15.4, 1.16.0rc1, 1.16.0rc2, 1.16.0, 1.16.1, 1.16.2, 1.16.3, 1.16.4, 1.16.5, 1.16.6, 1.17.0rc1, 1.17.0rc2, 1.17.0, 1.17.1, 1.17.2, 1.17.3, 1.17.4, 1.17.5, 1.18.0rc1, 1.18.0, 1.18.1, 1.18.2, 1.18.3, 1.18.4, 1.18.5, 1.19.0rc1, 1.19.0rc2, 1.19.0, 1.19.1, 1.19.2, 1.19.3, 1.19.4, 1.19.5"
"cp36-win_amd64 wheel","1.12.0, 1.12.1, 1.13.0rc1, 1.13.0rc2, 1.13.0, 1.13.1, 1.13.3, 1.14.0rc1, 1.14.0, 1.14.1, 1.14.2, 1.14.3, 1.14.4, 1.14.5, 1.14.6, 1.15.0rc1, 1.15.0rc2, 1.15.0, 1.15.1, 1.15.2, 1.15.3, 1.15.4, 1.16.0rc1, 1.16.0rc2, 1.16.0, 1.16.1, 1.16.2, 1.16.3, 1.16.4, 1.16.5, 1.16.6, 1.17.0rc1, 1.17.0rc2, 1.17.0, 1.17.1, 1.17.2, 1.17.3, 1.17.4, 1.17.5, 1.18.0rc1, 1.18.0, 1.18.1, 1.18.2, 1.18.3, 1.18.4, 1.18.5, 1.19.0rc1, 1.19.0rc2, 1.19.0, 1.19.1, 1.19.2, 1.19.3, 1.19.4, 1.19.5"
"cp37-win32 wheel","1.14.5, 1.14.6, 1.15.0rc2, 1.15.0, 1.15.1, 1.15.2, 1.15.3, 1.15.4, 1.16.0rc1, 1.16.0rc2, 1.16.0, 1.16.1, 1.16.2, 1.16.3, 1.16.4, 1.16.5, 1.16.6, 1.17.0rc1, 1.17.0rc2, 1.17.0, 1.17.1, 1.17.2, 1.17.3, 1.17.4, 1.17.5, 1.18.0rc1, 1.18.0, 1.18.1, 1.18.2, 1.18.3, 1.18.4, 1.18.5, 1.19.0rc1, 1.19.0rc2, 1.19.0, 1.19.1, 1.19.2, 1.19.3, 1.19.4, 1.19.5, 1.20.0rc1, 1.20.0rc2, 1.20.0, 1.20.1, 1.20.2, 1.20.3, 1.21.0rc1, 1.21.0rc2, 1.21.0, 1.21.1, 1.21.2, 1.21.3, 1.21.4"
"cp37-win_amd64 wheel","1.14.5, 1.14.6, 1.15.0rc2, 1.15.0, 1.15.1, 1.15.2, 1.15.3, 1.15.4, 1.16.0rc1, 1.16.0rc2, 1.16.0, 1.16.1, 1.16.2, 1.16.3, 1.16.4, 1.16.5, 1.16.6, 1.17.0rc1, 1.17.0rc2, 1.17.0, 1.17.1, 1.17.2, 1.17.3, 1.17.4, 1.17.5, 1.18.0rc1, 1.18.0, 1.18.1, 1.18.2, 1.18.3, 1.18.4, 1.18.5, 1.19.0rc1, 1.19.0rc2, 1.19.0, 1.19.1, 1.19.2, 1.19.3, 1.19.4, 1.19.5, 1.20.0rc1, 1.20.0rc2, 1.20.0, 1.20.1, 1.20.2, 1.20.3, 1.21.0rc1, 1.21.0rc2, 1.21.0, 1.21.1, 1.21.2, 1.21.3, 1.21.4"
"cp38-win32 wheel","1.17.3, 1.17.4, 1.17.5, 1.18.0rc1, 1.18.0, 1.18.1, 1.18.2, 1.18.3, 1.18.4, 1.18.5, 1.19.0rc1, 1.19.0rc2, 1.19.0, 1.19.1, 1.19.2, 1.19.3, 1.19.4, 1.19.5, 1.20.0rc1, 1.20.0rc2, 1.20.0, 1.20.1, 1.20.2, 1.20.3, 1.21.0rc1, 1.21.0rc2, 1.21.0, 1.21.1, 1.21.2, 1.21.3, 1.21.4"
"cp38-win_amd64 wheel","1.17.3, 1.17.4, 1.17.5, 1.18.0rc1, 1.18.0, 1.18.1, 1.18.2, 1.18.3, 1.18.4, 1.18.5, 1.19.0rc1, 1.19.0rc2, 1.19.0, 1.19.1, 1.19.2, 1.19.3, 1.19.4, 1.19.5, 1.20.0rc1, 1.20.0rc2, 1.20.0, 1.20.1, 1.20.2, 1.20.3, 1.21.0rc1, 1.21.0rc2, 1.21.0, 1.21.1, 1.21.2, 1.21.3, 1.21.4"
"cp39-win32 wheel","1.19.3, 1.19.4, 1.19.5, 1.20.0rc1, 1.20.0rc2, 1.20.0, 1.20.1, 1.20.2, 1.20.3, 1.21.0rc1, 1.21.0rc2, 1.21.0, 1.21.1, 1.21.2, 1.21.3, 1.21.4"
""cp39-win_amd64 wheel,""1.19.3, 1.19.4, 1.19.5, 1.20.0rc1, 1.20.0rc2, 1.20.0, 1.20.1, 1.20.2, 1.20.3, 1.21.0rc1, 1.21.0rc2, 1.21.0, 1.21.1, 1.21.2, 1.21.3, 1.21.4
"cp310-win32 wheel",""
"cp310-win_amd64 wheel","1.21.3, 1.21.4"

@seanlaw
Copy link
Collaborator Author

seanlaw commented Nov 9, 2021

@louisabraham As you can see, there isn't really a "true" minimum version of NumPy that would satisfy all operating systems. Can you please advise on what you'd want me to do?

@louisabraham
Copy link
Owner

louisabraham commented Nov 9, 2021 via email

@seanlaw
Copy link
Collaborator Author

seanlaw commented Nov 9, 2021

Let's do 1.19.5

Unfortunately, with numpy==1.19.5, I am still getting the same build error in Github Actions:

E   ValueError: numpy.ndarray size changed, may indicate binary incompatibility. Expected 88 from C header, got 80 from PyObject

I'm trying numpy==1.19 now (without the patch version number) - Update also doesn't work with the same error :(

@seanlaw
Copy link
Collaborator Author

seanlaw commented Nov 10, 2021

@louisabraham I'm encountering the same cibuildwheel issue that you've run into before:

E   OSError: [WinError 193] %1 is not a valid Win32 application

However, in my case (see Github Actions log), after building the cp36-win32 wheel (CPython 3.6 Windows 32bit), I encounter the above error trying to run the unit tests.

I don't know if this is a distraction but, in my Github Actions workflow, I don't have $PLATFORM_OPTION specified, which is referenced in your build.sh file. However, it looked like this environment variable is blank when dealing with win32 (which is wheel referenced above). I have no experience dealing with Windows and I'm hoping that you can provide some guidance here?

@louisabraham
Copy link
Owner

louisabraham commented Nov 10, 2021

Well I don't have a windows setup anymore so I'm not sure I can efficiently help. Isn't it working now?

I really want to provide binaries for windows as it is one major feature of pydivsufsort compared to other project: provide a one line installation of divsufsort on all platforms.

@seanlaw
Copy link
Collaborator Author

seanlaw commented Nov 10, 2021

Well I don't have a windows setup anymore so I'm not sure I can efficiently help.

Yeah, similarly, I work on a Mac and only have access to Windows via what is available on Github Actions.

Isn't it working now?

The wheels (built with cibuildwheels) seem to work for all of the Linux and MacOS platforms and unit tests all pass. However, for Windows, only amd64 wheels (built with cibuildwheels) are being built correctly with passing tests. I can't seem to build proper wheels for x86 on Github Actions and I'm not sure why it is different from TravisCI.

My hypothesis is that maybe for Github Actions I need to pass the -A win32 flag to cmake when building the wheels for x86 architecture? Whereas, in TravisCI, it was the opposite and we needed to pass in the -A x64 flag to cmake? Any thoughts would be appreciated!

I really want to provide binaries for windows as it is one major feature of pydivsufsort compared to other project: provide a one line installation of divsufsort on all platforms.

In case it matters, I understand and I am not advocating for the removal of Windows support. I just don't have any experience here and am debugging through Github Actions.

@Pierre-Sassoulas
Copy link

Would it be possible to create an env variable to give to the -A option so it's defined properly for each run?

@louisabraham
Copy link
Owner

louisabraham commented Nov 10, 2021 via email

@seanlaw
Copy link
Collaborator Author

seanlaw commented Nov 10, 2021

Would it be possible to create an env variable to give to the -A option so it's defined properly for each run?

@Pierre-Sassoulas That's what I'm thinking as well (and this is what is being done for TravisCI). However, I'm struggling (due to my inexperience) to figure out how I can set the env variable PLATFORM_OPTION="-A win32" for cibuildwheel if and only when it is building the Windows x86 wheels within its container. I couldn't find a cibuildwheel env variable that could tell me what the current architecture is at build time.

@seanlaw
Copy link
Collaborator Author

seanlaw commented Nov 10, 2021

I have posted the question with the cibuildwheel Github Discussions

@seanlaw
Copy link
Collaborator Author

seanlaw commented Nov 10, 2021

I can confirm that setting PLATFORM_OPTION="-A win32" now works for the x86 builds and the unit tests pass! I just need a way to be able to detect when it is x86 or x64 on windows

@seanlaw
Copy link
Collaborator Author

seanlaw commented Nov 10, 2021

Okay, I split the win32 and amd64 windows builds and we now have wheels (passing unit tests) for:

Windows AMD64
  pydivsufsort-0.0.5-cp310-cp310-win_amd64.whl
  pydivsufsort-0.0.5-cp36-cp36m-win_amd64.whl
  pydivsufsort-0.0.5-cp37-cp37m-win_amd64.whl
  pydivsufsort-0.0.5-cp38-cp38-win_amd64.whl
  pydivsufsort-0.0.5-cp39-cp39-win_amd64.whl

Windows x86
  pydivsufsort-0.0.5-cp310-cp310-win32.whl
  pydivsufsort-0.0.5-cp36-cp36m-win32.whl
  pydivsufsort-0.0.5-cp37-cp37m-win32.whl
  pydivsufsort-0.0.5-cp38-cp38-win32.whl
  pydivsufsort-0.0.5-cp39-cp39-win32.whl

MacOSX
  pydivsufsort-0.0.5-cp310-cp310-macosx_10_9_x86_64.whl
  pydivsufsort-0.0.5-cp36-cp36m-macosx_10_9_x86_64.whl
  pydivsufsort-0.0.5-cp37-cp37m-macosx_10_9_x86_64.whl
  pydivsufsort-0.0.5-cp38-cp38-macosx_10_9_x86_64.whl
  pydivsufsort-0.0.5-cp39-cp39-macosx_10_9_x86_64.whl

Linux
  pydivsufsort-0.0.5-cp310-cp310-manylinux_2_12_i686.manylinux2010_i686.whl
  pydivsufsort-0.0.5-cp310-cp310-manylinux_2_12_x86_64.manylinux2010_x86_64.whl
  pydivsufsort-0.0.5-cp310-cp310-musllinux_1_1_i686.whl
  pydivsufsort-0.0.5-cp310-cp310-musllinux_1_1_x86_64.whl
  pydivsufsort-0.0.5-cp36-cp36m-manylinux_2_12_i686.manylinux2010_i686.whl
  pydivsufsort-0.0.5-cp36-cp36m-manylinux_2_12_x86_64.manylinux2010_x86_64.whl
  pydivsufsort-0.0.5-cp36-cp36m-musllinux_1_1_i686.whl
  pydivsufsort-0.0.5-cp36-cp36m-musllinux_1_1_x86_64.whl
  pydivsufsort-0.0.5-cp37-cp37m-manylinux_2_12_i686.manylinux2010_i686.whl
  pydivsufsort-0.0.5-cp37-cp37m-manylinux_2_12_x86_64.manylinux2010_x86_64.whl
  pydivsufsort-0.0.5-cp37-cp37m-musllinux_1_1_i686.whl
  pydivsufsort-0.0.5-cp37-cp37m-musllinux_1_1_x86_64.whl
  pydivsufsort-0.0.5-cp38-cp38-manylinux_2_12_i686.manylinux2010_i686.whl
  pydivsufsort-0.0.5-cp38-cp38-manylinux_2_12_x86_64.manylinux2010_x86_64.whl
  pydivsufsort-0.0.5-cp38-cp38-musllinux_1_1_i686.whl
  pydivsufsort-0.0.5-cp38-cp38-musllinux_1_1_x86_64.whl
  pydivsufsort-0.0.5-cp39-cp39-manylinux_2_12_i686.manylinux2010_i686.whl
  pydivsufsort-0.0.5-cp39-cp39-manylinux_2_12_x86_64.manylinux2010_x86_64.whl
  pydivsufsort-0.0.5-cp39-cp39-musllinux_1_1_i686.whl
  pydivsufsort-0.0.5-cp39-cp39-musllinux_1_1_x86_64.whl

@seanlaw
Copy link
Collaborator Author

seanlaw commented Nov 10, 2021

@louisabraham I think this PR is ready for your review

@louisabraham
Copy link
Owner

That's AWESOME !!!

Thank you so much !!!

There is still the question of M1 binaries but let's wait for a more stable support in CIBW.

I'll have a look at pypi upload

@louisabraham louisabraham merged commit 59a2d87 into louisabraham:master Nov 10, 2021
@seanlaw
Copy link
Collaborator Author

seanlaw commented Nov 11, 2021

That's AWESOME !!!
Thank you so much !!!

No, thank you for making this package available! I've enjoyed our collaboration

There is still the question of M1 binaries but let's wait for a more stable support in CIBW.

Agreed

I'll have a look at pypi upload

If you look at this new github-actions.yml file (it's very similar to the .travis.yml file), grep for all of the places where the word twine comes up and those need to be changed accordingly

Also, I didn't get a chance to add the code coverage so I hope that you can take a look. Right now, every PR or commit will trigger this Github Actions workflow so you might want to set up different workflows (i.e., for releases, for checking PRs, etc)

@seanlaw seanlaw changed the title [WIP - Do Not Merge] Initial Migration to Github Actions Initial Migration to Github Actions Nov 12, 2021
@Alexander-Serov
Copy link
Contributor

Alexander-Serov commented Nov 12, 2021

Thanks, great job guys!

As for numpy version, from what I understood from cython guys answers is that you just need to find the maximal of the minimums for all the platforms of interest. Judging by your log, it is 1.19 if you want to support py39 and up only, or 1.17 for py38+, or 1.14 for py37+ (it is surprising the build with the config I changed to 1.14 failed).

Note also that this is the numpy version you only need for the build. It is not a formal requirement of the package because it just fixes the C interface (according to the answer reposted by @louisabraham )

@Alexander-Serov
Copy link
Contributor

Also, I have access to the Windows platform. Let me know what you need to be tested.

@seanlaw
Copy link
Collaborator Author

seanlaw commented Nov 12, 2021

As for numpy version, from what I understood from cython guys answers is that you just need to find the maximal of the minimums for all the platforms of interest. Judging by your log, it is 1.19 if you want to support py39 and up only, or 1.17 for py38+, or 1.14 for py37+ (it is surprising the build with the config I changed to 1.14 failed).

We will allow oldest-supported-numpy to handle this since there are platforms/architectures that have a minimum numpy version of 1.20.x (see #34 (comment)). Using oldest-supported-numpy for building all of our platforms/architectures will lead to the least amount of failures and the maximum compatibility.

@Alexander-Serov
Copy link
Contributor

Perfect, thank you! @louisabraham has explained the same thing!
So do I understand it correctly that this is merged and everything is building OK for all platforms? Or do you still need something for x86?

Does this merge close your issue #31 ?

@Alexander-Serov
Copy link
Contributor

Alexander-Serov commented Nov 12, 2021

Are the wheels updated in the repo? Because returning to my original issue #30, the error in my setup is the same with the latest wheels found by pip:

Collecting pydivsufsort>=0.0.5
  Downloading pydivsufsort-0.0.5-cp38-cp38-manylinux_2_12_x86_64.manylinux2010_x86_64.whl (1.4 MB)

ERROR
/usr/local/lib/python3.8/site-packages/pydivsufsort/__init__.py:2: in <module>
    from .stringalg import (
pydivsufsort/stringalg.pyx:1: in init pydivsufsort.stringalg
    ???
E   ValueError: numpy.ndarray size changed, may indicate binary incompatibility. Expected 88 from C header, got 80 from PyObject

@seanlaw
Copy link
Collaborator Author

seanlaw commented Nov 12, 2021

Does this merge close your issue #31 ?

Yes. Manually closed now. Thank you!

Are the wheels updated in the repo? Because returning to my original issue #30, the error in my setup is the same with the latest wheels found by pip:

We have limited this PR for migrating from TravisCI to Github Actions for building our wheels. The final step of uploading the wheels/sdist to PyPI is being tracked in #37.

To answer your question, the new wheels are not available on PyPI yet.

@louisabraham
Copy link
Owner

I have to configure the uploading to PyPI myself. I'll try to find time for this over the WE.

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.

None yet

4 participants