search

PyWavelets / pywt

PyWavelets - Wavelet Transforms in Python
http://pywavelets.readthedocs.org
MIT License
1.97k stars 460 forks source link

Upload dev wheels to Anaconda.org + revamp wheels publishing workflow #714

Closed agriyakhetarpal closed 3 months ago

agriyakhetarpal commented 3 months ago

Description

This PR introduces the following changes:

  1. It adds a deploy_anaconda job where the wheels can be uploaded to https://anaconda.org/scientific-python-nightly-wheels/PyWavelets/ using the scientific-python/upload-nightly-action GitHub Action
  2. A workflow_dispatch trigger to push the nightly wheels, and a CRON schedule that matches the one in #710
  3. Replaces the cibuildwheel installation with its upstream GitHub Action, in order to get updates from Dependabot (see #708)
  4. Bumps up versions for the checkout actions and bumps the major version for download-artifact and upload-artifact
  5. Ensures that the necessary job runs or is skipped, based on the other wheel builds that may pass or fail

Footnotes

This PR is related to the changes requested on #712.

agriyakhetarpal commented 3 months ago

@rgommers, this is ready for your review whenever you have the time for it. I thought that the changes weren't much!

agriyakhetarpal commented 3 months ago

With these changes, the PyPI job will run on:

  1. Tags

and the Anaconda PyPI index job will run on:

  1. Pushes to master/v1.XX,
  2. On a schedule, same as the WASM upload job (should we alter the CRON statement to have a 5-minute gap in case too many jobs start?)
  3. Manually

Should we allow the PyPI job to be triggered manually as well?

rgommers commented 3 months ago

Should we allow the PyPI job to be triggered manually as well?

Yes, that would be useful to do.

agriyakhetarpal commented 3 months ago

I guess we should add another input to the workflow_dispatch: event to gauge where to upload the wheels (there might be a situation where we would want to publish to just Anaconda, or just PyPI, or both).

rgommers commented 3 months ago

I guess we should add another input to the workflow_dispatch: event to gauge where to upload the wheels (there might be a situation where we would want to publish to just Anaconda, or just PyPI, or both).

You are right. Actually, on second thought, this is a little dangerous. It should be a little hard to deploy to PyPI. Let's drop that last change. We do like 1-2 releases a year, so in case there's a problem with building from the tag, let's just make sure a maintainer fixes the problem and then moves the tag to do a release.

agriyakhetarpal commented 3 months ago

You are right. Actually, on second thought, this is a little dangerous. It should be a little hard to deploy to PyPI. Let's drop that last change. We do like 1-2 releases a year, so in case there's a problem with building from the tag, let's just make sure a maintainer fixes the problem and then moves the tag to do a release.

Thanks for the clarification! Yes, PyPI is permanent, so we do not want to trigger a broken release (even if the input is false by default). I have reverted the change in https://github.com/PyWavelets/pywt/pull/714/commits/e8e4d86f42aab0fe1a011f8496b9cb53e1a74033.

rgommers commented 3 months ago

I tried this on my fork, and the uploading is broken: https://github.com/rgommers/pywt/actions/runs/8252417966. The problem is the name: field of upload-artifact, it is not specific enough. Maybe it used to work and the action got more strict.

On other projects I see that there is one wheel per zip file with the Python interpreter included in the upload name, e.g.: https://github.com/numpy/numpy/actions/runs/8238976297. I'm not sure if that is the optimal solution. Could you investigate, and then test from the master branch of your own fork?

agriyakhetarpal commented 3 months ago

Yes, it used to be more lenient with v3. I checked the workflow and the reason is that we are restricting the Python versions being built using cibw_python in the matrix – this produces parallel jobs per Python version (and therefore the name of the artifact must be different and the matrix variable must be present).

I had not used the CIBW_BUILD environment variable before and therefore did not face this problem (the difference is that it would build Python 3.9–3.12 in serial, one after the other, in the same job).

I have pushed https://github.com/PyWavelets/pywt/pull/714/commits/90ec5e4cbf8d4bbf0aaf8c224df0ef2abe714f3d which should fix this, here's a workflow run I have triggered just now – let's see if it passes: https://github.com/agriyakhetarpal/pywt/actions/runs/8252934295

rgommers commented 3 months ago

Looks like it doesn't like the * in the name.

agriyakhetarpal commented 3 months ago

Yes, https://github.com/agriyakhetarpal/pywt/actions/runs/8253018855 works and all wheel builds plus their uploads are passing.

agriyakhetarpal commented 3 months ago

Now that both cibuildwheel and meson-python correctly set MACOSX_DEPLOYMENT_TARGET, we should look to build macOS arm64 wheels and test them natively before cutting 1.6.0. Do you mind opening a separate issue and PR for that or can I make that change here?

Edit: based on our Slack conversation, I shall do this and MUSL wheels in a follow-up PR.

rgommers commented 3 months ago

The upload to anaconda.org didn't actually work: https://github.com/PyWavelets/pywt/actions/runs/8253753639/job/22576823450. Could you have a look at that?

rgommers commented 3 months ago

Ah never mind, you're already on it!