Skip to content

CI: use pipx and build to simplify and remove a workaround #43157

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

CI: use pipx and build to simplify and remove a workaround #43157

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
71b80b8
CI: use pipx and build to simplify and remove a workaround
henryiii Aug 21, 2021
29f45b7
ci: drop setup-python, show build version
henryiii Oct 5, 2021
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
16 changes: 2 additions & 14 deletions .github/workflows/sdist.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,22 +33,10 @@ jobs:
with:
fetch-depth: 0

- name: Set up Python
uses: actions/setup-python@v2
with:
python-version: ${{ matrix.python-version }}

- name: Install dependencies
run: |
Copy link
Contributor

Choose a reason for hiding this comment

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

the idea is to simulate core building and distributing this application. how does this improve the experience here?

Copy link
Author

@henryiii henryiii Aug 21, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Artifact building should happen with build, as well. build will respect PEP 518 requirements, raw setup.py commands cannot, which is why the workaround in #39416 was introduced. Making an SDist should be done with pip install build && python -m build --dist (or pipx run build --sdist), not pip install setuptools wheel numpy && python setup.py sdist --formats=gztar.

Copy link
Contributor

Choose a reason for hiding this comment

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

can you replace with pip install build && python -m build --dist prefer to use the standard tools here

Copy link
Author

Choose a reason for hiding this comment

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

pipx is a standard tool, it's part of the PyPA, just like build, setuptools, and pip. I can change this if preferred, but then you are dependent on setup-python@v2, while pipx is not.

python -m pip install --upgrade pip setuptools wheel

# GH 39416
pip install numpy

- name: Build pandas sdist
run: |
pip list
Copy link
Member

Choose a reason for hiding this comment

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

i think we usually want this line so that we can see what versions are installed when debugging CI failures

Copy link
Author

Choose a reason for hiding this comment

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

This is not the build environment. No version listed here would have anything to do with what you have inside the PEP 517 build environment. If you use pipx, as below, then even the version of build is not listed and the version of Python is unrelated.

Copy link
Author

Choose a reason for hiding this comment

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

I've dropped setup-python, which should reduce confusion here, and added a --version run so that the logs will have the build version.

python setup.py sdist --formats=gztar
pipx run build --version
pipx run build --sdist

- uses: conda-incubator/setup-miniconda@v2
with:
Expand Down