Skip to content

Release Workflow for CI#320

Merged
rkingsbury merged 7 commits intoKingsburyLab:mainfrom
vineetbansal:main
Jan 29, 2026
Merged

Release Workflow for CI#320
rkingsbury merged 7 commits intoKingsburyLab:mainfrom
vineetbansal:main

Conversation

@vineetbansal
Copy link
Copy Markdown
Collaborator

@vineetbansal vineetbansal commented Jan 26, 2026

Summary

Thanks for merging the PRs leading up to this.

I've tried to streamline the release process, mostly based on what usually works best in most Python projects I've worked with (and because I don't exactly know what some of the marketplace actions that were used before are doing under the hood).

Some changes for CI optimizations are:

  • Because our tests take so long (around an hour on each python/OS combo), the testing workflow now tests our code only on Linux. I feel this is a good base platform for detecting regressions. This has the effect of detecting any regressions with newer versions of our dependencies. We test all supported python versions to ensure that our code is not using language constructs that are only applicable to a particular python version (and neither are any of our dependencies).
  • The testing-pinned workflow takes the responsibility of detecting platform-specific nuances. We run all tests using pinned dependencies on our minimum and maximum supported python versions on all platforms. This tests our own code running on the latest and greatest python versions on all platforms, while making sure we're not relying on new features of the language on any platform, while taking our dependencies out of the equation. This workflow is thus a proxy now for anything we may have done to our code to break it.

However, the above optimizations are entirely my opinion - we can put back a common/larger matrix for simplicity and everything will still work, though it will take longer.

Other changes:

  • The release workflow now runs when PRs are merged to main (or on direct pushes to main, which we don't allow for now anyway). pyEQL is built inside isolated containers using cibuildwheel, and the tests related to our C++ code ( intests/phreeqc folder) are run inside the container. Again, this is an optimization and we can run all tests if we want, but I think we don't need them because of the testing.yml and testing-pinned.yaml workflows which detect other failure modes anyway. In most hybrid Python/C++ projects, its usually sufficient to test if import <project-name> works inside the container and move on, so we're doing slightly better than that anyway.

  • All artifacts (sdist and binary wheels) are collected as github artifacts so we can inspect them if necessary.

  • If, in addition, a tag has been applied to the commit on the main branch, then we use setuptools-scm (like before) to make that tag the version number of pyEQL. (v1.3.2 was the last one, we can go with v1.4.0 next), and use pypi_password project secret (which is essentially the API key you get from PyPI), to upload the final wheels to PyPI. You will have to set this secret up with this exact name, though you likely have some version of it in there already.

I've tested out everything (except the final release to PyPI part of course) on a fork of this repo. I guess we'll find out if something fails when we merge - which is why we should probably tag the next release as v1.4.0-pre0 or v1.4.0.rc0 instead of using a real version number. If we merge this but not apply a tag then everything will run except the final release-to-pypi step.

Happy to tweak or answer any concerns!

Checklist

  • Google format doc strings added.
  • Code linted with ruff. (For guidance in fixing rule violates, see rule list)
  • Type annotations included. Check with mypy.
  • Tests added for new features/fixes.
  • I have run the tests locally and they passed.

Tip: Install pre-commit hooks to auto-check types and linting before every commit:

pip install -U pre-commit
pre-commit install

@codecov
Copy link
Copy Markdown

codecov bot commented Jan 26, 2026

Codecov Report

❌ Patch coverage is 50.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.43%. Comparing base (16dad56) to head (d60d247).
⚠️ Report is 9 commits behind head on main.

Files with missing lines Patch % Lines
src/pyEQL/__init__.py 33.33% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #320      +/-   ##
==========================================
- Coverage   86.54%   86.43%   -0.11%     
==========================================
  Files          14       14              
  Lines        1850     1851       +1     
  Branches      320      320              
==========================================
- Hits         1601     1600       -1     
- Misses        205      207       +2     
  Partials       44       44              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Member

@rkingsbury rkingsbury left a comment

Choose a reason for hiding this comment

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

Looks great @vineetbansal , thank you. I'm happy to be moving away from 3rd party GitHub actions; I just wasn't aware of the "stock" alternatives.

One question - prior to your work we were using uv pip install with a resolution kwarg to test the lowest and highest supported versions of the dependencies in pyproject.toml (in the post-process workflow I believe, not in individual PRs). The idea behind this was to catch cases where a change we made in pyEQL made use of some new feature of a package or something, causing an "officially support" as per pyproject.toml version to fail.

By now testing against requirements.txt, I think we will miss those cases. Do you see any value in adding that back in? I think your approach will functionally test the highest supported versions (which I think is what winds up in requirements.txt, but probably not the lowest.

Apart from that and two minor comments, I'm ready to merge. I'm also happy to merge now if you'd prefer to deal with any comments in a separate PR.

Comment on lines 6 to 8
- main
- pourbaix
paths-ignore:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please keep the pourbaix branch here. We have some active development going on there (or we will soon) and I want the workflow to run on PRs against that branch.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Added back in.

@rkingsbury
Copy link
Copy Markdown
Member

we should probably tag the next release as v1.4.0-pre0 or v1.4.0.rc0

Question here - in the old (3rd party) workflow, the tags that triggered a release were release:minor and release:major. Here, will the PyPi release only happen if the tag "looks like" a version number? e.g. v1.4.0rc0? Or will any tag trigger a PyPi release?

@vineetbansal
Copy link
Copy Markdown
Collaborator Author

vineetbansal commented Jan 29, 2026

@rkingsbury - good point about the tag. The previous iteration would have initiated the release process on any tag on the main branch. This works fine for most projects but I've expanded the criteria in our tagcheck step in the detect_tag job, so we're only doing this on tags that look like versions:

      - id: tagcheck
        run: |
          TAG=$(git tag --points-at HEAD)
          if [[ "$TAG" =~ ^v?[0-9]+\.[0-9]+\.[0-9]+ ]]; then
            echo "Detected tag: $TAG"
            echo "tag=$TAG" >> $GITHUB_OUTPUT
          fi

@vineetbansal
Copy link
Copy Markdown
Collaborator Author

@rkingsbury - to answer your other questions:

One question - prior to your work we were using uv pip install with a resolution kwarg to test the lowest and highest supported versions of the dependencies in pyproject.toml (in the post-process workflow I believe, not in individual PRs). The idea behind this was to catch cases where a change we made in pyEQL made use of some new feature of a package or something, causing an "officially support" as per pyproject.toml version to fail.

By now testing against requirements.txt, I think we will miss those cases. Do you see any value in adding that back in? I think your approach will functionally test the highest supported versions (which I think is what winds up in requirements.txt, but probably not the lowest.

I think its most useful to test scenarios that the end users (or developers) will be running themselves - either a pip install or a pip install -r requirements.txt. The resolution flag is useful debugging tool for when things do fail (in the testing.yml workflow, not the testing-pinned.yml workflow), but only useful locally where one can quickly iterate over several versions and pinpoint to what caused a test to fail (and then encode the dependency in a more fine-grained way in pyproject.toml, as well as update requirements.txt). Having this in the CI will cause headaches with no discernible pattern to the errors, since no python project nowadays sticks to the semver system anyway.

Of course this is my opinion based on several recent python projects I've been involved with. It's very possible that I haven't discovered the true values of these flags while others have. But even then I'd probably handle that as a separate PR, where the goal is to make the project "uv-first" (with a uv.lock file and checking the CI in this fine-grained way with uv pip).

Copy link
Copy Markdown
Member

@rkingsbury rkingsbury left a comment

Choose a reason for hiding this comment

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

Awesome, thanks for the detailed explanations. I trust your judgment on this!

@rkingsbury rkingsbury added pkg Package and repository health github_actions Pull requests that update GitHub Actions code labels Jan 29, 2026
@rkingsbury rkingsbury merged commit 21cc3c2 into KingsburyLab:main Jan 29, 2026
14 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

github_actions Pull requests that update GitHub Actions code pkg Package and repository health

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants