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

Use Poetry for dependency management #148

Merged
merged 20 commits into from
Dec 15, 2023
Merged
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
6 changes: 3 additions & 3 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@ on: push
jobs:
black:

runs-on: ubuntu-20.04
runs-on: ubuntu-22.04

steps:
- uses: actions/checkout@v2
- uses: actions/setup-python@v2
- uses: actions/checkout@v4
- uses: actions/setup-python@v4
Copy link
Contributor

Choose a reason for hiding this comment

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

The existence of v4 is worth mentioning to all of us in Zulip or at the next team meeting. Are there any noticeable differences?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't research any differences... just saw a deprecation notice w.r.t. v2 so I upgraded to the most recent version. It's probably worth mentioning to folks, even if I'm not very well informed about it aside from "it works!".

- uses: psf/black@stable
Copy link
Contributor

@rod-glover rod-glover Dec 14, 2023

Choose a reason for hiding this comment

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

I've found it better to install Black (with a specific version) as a dev dependency on and invoke it using Python in the workflow script and from the command line or IDE when developing. Otherwise different versions of Black disagree on their defaults (@stable does change), and it is very frustrating. Further, different repos following this patter use different versions, so it is not possible at present to have one global version on your machine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you point to a similar config in another repo that we could adopt in this one? (I'm not finding a lint.yml in any other repositories).

My approach to using black has essentially been asking it to fix our formatting for us, and its version hasn't been of any consequence.

Copy link
Contributor

@rod-glover rod-glover Dec 15, 2023

Choose a reason for hiding this comment

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

It is possible that Black has settled down lately, but in the past it changed its default settings from release to release. Which caused angst when it changed under you -- potentially hundreds of changes in a formerly Black-compliant codebase. Hence version pinning. And making it part of the dev deps, so that everyone working on the repo, plus the workflow, is using the same Black, always.

I've noticed that Black checks are often the last step of the Python CI workflow. For example, in pycds. I believe this is because a failing Black check can cause your unit tests to be cancelled, and this can be undesirable. I think I recall discussing this with Nik a while back when we were both working on the same repo. In any case, it is common practice in our projects.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd argue that it's a good thing for black checks to be done first since they are the cheapest and that we consider them to be important for controlling code quality.

If there is a specific version of black that we should pin to, please feel free to recommend it :)

with:
black_args: ". --check"
27 changes: 16 additions & 11 deletions .github/workflows/pypi-publish.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,24 +8,29 @@ on:
jobs:
publish:

runs-on: ubuntu-20.04
runs-on: ubuntu-22.04

steps:
- uses: actions/checkout@v2
- uses: actions/checkout@v4
- name: Set up Python
uses: actions/setup-python@v2
uses: actions/setup-python@v4
with:
python-version: '3.8'

- name: Install dependencies
- name: Install Poetry
run: |
python -m pip install --upgrade pip
pip install setuptools wheel twine

curl -sSL https://install.python-poetry.org | python3 -

- name: Build and publish
env:
TWINE_USERNAME: ${{ secrets.pcic_at_pypi_username }}
TWINE_PASSWORD: ${{ secrets.pcic_at_pypi_password }}
PCIC_PYPI_USERNAME: ${{ secrets.pcic_at_pypi_username }}
PCIC_PYPI_PASSWORD: ${{ secrets.pcic_at_pypi_password }}
run: |
python setup.py sdist bdist_wheel
twine upload --repository-url https://pypi.pacificclimate.org/ --skip-existing -u $TWINE_USERNAME -p $TWINE_PASSWORD dist/*
# Configure Poetry to publish to PCIC private package repository
# Private repo name is "pcic". We could factor that out as an env var...
poetry config repositories.pcic https://pypi.pacificclimate.org/
poetry config http-basic.pcic $PCIC_PYPI_USERNAME $PCIC_PYPI_PASSWORD
jameshiebert marked this conversation as resolved.
Show resolved Hide resolved

# Build and publish
poetry build
poetry publish -r pcic
33 changes: 11 additions & 22 deletions .github/workflows/python-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,15 @@ on: push
jobs:
test:

runs-on: ubuntu-20.04
runs-on: ubuntu-22.04
strategy:
matrix:
python-version: [3.6, 3.7, 3.8]
python-version: [3.8, 3.9]

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

Expand All @@ -23,32 +23,21 @@ jobs:
sudo apt-get install libhdf5-serial-dev netcdf-bin libnetcdf-dev
sudo apt-get install cdo

- name: Install pipenv
- name: Install poetry
run: |
pip install -U pipenv

- id: cache-pipenv
uses: actions/cache@v2
with:
path: ~/.local/share/virtualenvs
key: ${{ runner.os }}-pipenv-${{ hashFiles('**/Pipfile.lock') }}
wget -O - https://install.python-poetry.org | python3 -
echo "$HOME/.local/bin" >> $GITHUB_PATH

- name: Install dependencies if changed
if: steps.cache-pipenv.outputs.cache-hit != 'true' && ${{ matrix.python-version == '3.8' }}
run: |
pipenv install --deploy --dev
- name: Re-install dependencies if alternative python version
if: ${{ matrix.python-version != '3.8' }}
- name: Install python dependencies
run: |
mv Pipfile.lock do-not-use
pipenv install --python ${{ matrix.python-version }} --dev
poetry install --with=dev

- name: Test with pytest (full)
if: github.ref == 'refs/heads/master'
run: |
pipenv run py.test -m "not online" -v
poetry run py.test -m "not online" -v

- name: Test with pytest (fast)
if: github.ref != 'refs/heads/master'
run: |
pipenv run py.test -m "not online and not slow" -v
poetry run py.test -m "not online and not slow" -v
25 changes: 0 additions & 25 deletions Pipfile

This file was deleted.

Loading
Loading