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

Added a ci-wheels yml #135

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 120 additions & 0 deletions .github/workflows/ci-wheels.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
# Reference:
# - https://github.com/actions/checkout
# - https://github.com/actions/download-artifact
# - https://github.com/actions/upload-artifact
# - https://github.com/pypa/cibuildwheel
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're not using cibuildwheel

# - https://github.com/pypa/build
# - https://github.com/pypa/gh-action-pypi-publish
# - https://test.pypi.org/help/#apitoken

name: ci-wheels

on:
pull_request:

push:
branches:
- "main"
- "v*x"
- "!auto-update-lockfiles"
- "!pre-commit-ci-update-config"
- "!dependabot/*"
tags:
- "v*"

jobs:
build_bdist:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This job is not currently creating a built distribution ("bdist") - it only has 1 step - checking out the repo.

name: "build ${{ matrix.os }} (${{ matrix.arch }}) wheels"
runs-on: ${{ matrix.os }}
strategy:
fail-fast: false
matrix:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like Tephi is agnostic to OS - there are no OS-specific files here:

https://pypi.org/project/tephi/#files 1

So the matrix should be unnecessary. E.g. ci-wheels.yml in Iris does not have one.

Footnotes

  1. Compare this to stratify, which has many OS-specific files.

os: ["ubuntu-latest", "macos-latest", "windows-latest"]
arch: ["x86_64", "arm64"]
exclude:
- os: "ubuntu-latest"
arch: "arm64"
- os: "windows-latest"
arch: "x86_64"
- os: "windows-latest"
arch: "arm64"
include:
- os: "windows-latest"
arch: "AMD64"

steps:
- uses: actions/checkout@v4
with:
fetch-depth: 0

build_sdist:
name: "Build sdist"
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
with:
fetch-depth: 0

- name: "Building sdist"
shell: bash
run: |
pipx run build --sdist

- uses: actions/upload-artifact@v4
with:
name: pypi-sdist
path: ${{ github.workspace }}/dist/*.tar.gz


show-artifacts:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Several other repos include a test stage. Looks like they download the artifacts and make sure that each provides a functional instance of the package.

Repos with this:

  • Iris
  • Iris-esmf-regrid
  • nc-time-axis

Repos without this:

  • Stratify
  • cf-units

It looks like the only times that we skip this are for repos that include Cython (and therefore build using cibuildwheel). Tephi is not in this camp, so could it include a testing stage?

needs: [build_bdist, build_sdist]
name: "Show artifacts"
runs-on: ubuntu-latest
steps:
- uses: actions/download-artifact@v4
with:
merge-multiple: true
path: ${{ github.workspace }}/dist

- shell: bash
run: |
ls -l ${{ github.workspace }}/dist


publish-artifacts-test-pypi:
needs: [build_bdist, build_sdist]
name: "Publish to Test PyPI"
runs-on: ubuntu-latest
# upload to Test PyPI for every commit on main branch
if: github.event_name == 'push' && github.event.ref == 'refs/heads/main'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In other repos you have suggested a check for the repository owner, too: SciTools/iris#5180. Can that be included here too?

steps:
- uses: actions/download-artifact@v4
with:
merge-multiple: true
path: ${{ github.workspace }}/dist

- uses: pypa/gh-action-pypi-publish@release/v1
with:
user: __token__
password: ${{ secrets.TEST_PYPI }}
repository_url: https://test.pypi.org/legacy/
skip_existing: true
print_hash: true

publish-artifacts-pypi:
needs: [build_bdist, build_sdist]
name: "Publish to PyPI"
runs-on: ubuntu-latest
# upload to PyPI for every tag starting with 'v'
if: github.event_name == 'push' && startsWith(github.event.ref, 'refs/tags/v')
steps:
- uses: actions/download-artifact@v4
with:
merge-multiple: true
path: ${{ github.workspace }}/dist

- uses: pypa/gh-action-pypi-publish@release/v1
with:
user: __token__
password: ${{ secrets.PYPI }}
print_hash: true
3 changes: 1 addition & 2 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,4 @@ requires = [
"wheel",
]
# Defined by PEP 517
build-backend = "setuptools.build_meta"

build-backend = "setuptools.build_meta"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change looks unintended