Skip to content

Refactor GitHub Actions for Python publishing#190

Merged
singjc merged 1 commit intomasterfrom
singjc-patch-2
Mar 2, 2026
Merged

Refactor GitHub Actions for Python publishing#190
singjc merged 1 commit intomasterfrom
singjc-patch-2

Conversation

@singjc
Copy link
Copy Markdown
Contributor

@singjc singjc commented Mar 2, 2026

No description provided.

Copilot AI review requested due to automatic review settings March 2, 2026 17:23
@singjc singjc merged commit fd90bf3 into master Mar 2, 2026
1 check failed
@singjc singjc deleted the singjc-patch-2 branch March 2, 2026 17:25
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Refactors the GitHub Actions workflow responsible for publishing the project’s Python package on GitHub Releases, aiming to build and upload distributions across multiple Python versions.

Changes:

  • Renames the publish job and introduces a Python-version matrix (3.9–3.13).
  • Builds both sdist and wheel distributions during the workflow run.
  • Publishes wheels for each matrix entry and publishes the sdist once via a matrix conditional.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +30 to +43
# Build sdist and wheel for each version
- name: Build distributions
run: |
python setup.py sdist
python setup.py bdist_wheel

# Upload wheel for each Python version
- name: Publish wheel
env:
TWINE_USERNAME: ${{ secrets.PYPI_USERNAME }}
TWINE_PASSWORD: ${{ secrets.PYPI_PASSWORD }}
run: |
twine upload dist/*.whl

Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

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

This job uploads Linux wheels built directly on ubuntu-latest via setup.py bdist_wheel. Those wheels will be tagged for the runner’s platform/glibc and can be incompatible with many user Linux environments; publishing them to PyPI can cause pip to prefer a wheel that later fails to import/run. Consider switching to a manylinux-based wheel build (e.g., cibuildwheel) or publishing only sdist unless you’re also producing portable wheels (and ideally for macOS/Windows too).

Suggested change
# Build sdist and wheel for each version
- name: Build distributions
run: |
python setup.py sdist
python setup.py bdist_wheel
# Upload wheel for each Python version
- name: Publish wheel
env:
TWINE_USERNAME: ${{ secrets.PYPI_USERNAME }}
TWINE_PASSWORD: ${{ secrets.PYPI_PASSWORD }}
run: |
twine upload dist/*.whl
# Build sdist for each version
- name: Build sdist
run: |
python setup.py sdist

Copilot uses AI. Check for mistakes.

# Upload sdist only once (e.g., on the oldest version)
- name: Publish sdist
if: matrix.python-version == '3.10'
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

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

The comment says the sdist is uploaded "only once (e.g., on the oldest version)", but the condition uses Python 3.10 even though the matrix includes 3.9. Either update the condition to match the intended "oldest" version or adjust the comment so future readers don’t get misled.

Suggested change
if: matrix.python-version == '3.10'
if: matrix.python-version == '3.9'

Copilot uses AI. Check for mistakes.
- name: Install dependencies
run: |
python -m pip install --upgrade pip numpy cython
pip install setuptools wheel twine
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

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

In matrix workflows it’s safer to consistently use python -m pip ... instead of calling pip directly, to ensure you’re installing into the interpreter selected by setup-python for that matrix entry.

Suggested change
pip install setuptools wheel twine
python -m pip install setuptools wheel twine

Copilot uses AI. Check for mistakes.
Comment on lines +38 to +41
env:
TWINE_USERNAME: ${{ secrets.PYPI_USERNAME }}
TWINE_PASSWORD: ${{ secrets.PYPI_PASSWORD }}
run: |
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

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

This workflow authenticates Twine using username/password-style secrets. If these are real credentials (not an API token), that’s a higher-risk secret with broad account access. Prefer using a PyPI API token (TWINE_USERNAME=token) or GitHub’s PyPI trusted publishing (OIDC) to avoid long-lived passwords.

Copilot uses AI. Check for mistakes.
Comment on lines +18 to +21
- uses: actions/checkout@v1

- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v1
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

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

actions/checkout@v1 and actions/setup-python@v1 are very old and inconsistent with the rest of this repo’s workflows (e.g., .github/workflows/ci.yml and build-release.yml use checkout@v4 and setup-python@v5). Updating to the newer major versions avoids relying on deprecated Node runtimes and gets the current action security fixes.

Suggested change
- uses: actions/checkout@v1
- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v1
- uses: actions/checkout@v4
- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v5

Copilot uses AI. Check for mistakes.
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.

2 participants