Skip to content

Fix the postprocess workflow; refactor PHREEQC2026 wrapper#318

Merged
rkingsbury merged 10 commits intomainfrom
vb/postprocess
Jan 23, 2026
Merged

Fix the postprocess workflow; refactor PHREEQC2026 wrapper#318
rkingsbury merged 10 commits intomainfrom
vb/postprocess

Conversation

@vineetbansal
Copy link
Copy Markdown
Collaborator

@vineetbansal vineetbansal commented Jan 20, 2026

I noticed that the post-process workflow for pyEQL (on which the release workflow depends) is currently broken. This PR attempts to fix it.

This is a WIP - I'm triggering the post-process workflow on all PRs to main (so we can fix this before merging it), not just merged ones, and running all steps except the final publish-to-pypi in the release workflow. These two changes will be reverted once everything else works.

@vineetbansal
Copy link
Copy Markdown
Collaborator Author

vineetbansal commented Jan 20, 2026

@rkingsbury - perhaps we can revisit permissions here. I'm getting a:

You're not authorized to push to this branch. Visit https://docs.github.com/repositories/configuring-branches-and-merges-in-your-repository/managing-protected-branches/about-protected-branches for more information.

Of course I'm not trying to push to main, just submit a PR to main. The current permissions might be too restrictive, and should be fixable in the settings page.

If the fix is not obvious and you're okay with it, perhaps you can give me temporary admin access till we fix this part.

@rkingsbury
Copy link
Copy Markdown
Member

Thanks for checking on this @vineetbansal ! Fingers crossed it works.

I've just upgraded your role to Maintain (from write) which I hope fixes the branch protection problem.

@codecov
Copy link
Copy Markdown

codecov bot commented Jan 20, 2026

Codecov Report

❌ Patch coverage is 80.00000% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.59%. Comparing base (07e2d9b) to head (5e9af0f).
⚠️ Report is 12 commits behind head on main.

Files with missing lines Patch % Lines
src/pyEQL/engines.py 60.00% 1 Missing and 1 partial ⚠️
src/pyEQL/phreeqc/core.py 83.33% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #318      +/-   ##
==========================================
+ Coverage   85.68%   86.59%   +0.90%     
==========================================
  Files          10       14       +4     
  Lines        1607     1842     +235     
  Branches      285      320      +35     
==========================================
+ Hits         1377     1595     +218     
- Misses        194      203       +9     
- Partials       36       44       +8     

☔ 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.

@vineetbansal
Copy link
Copy Markdown
Collaborator Author

vineetbansal commented Jan 22, 2026

@rkingsbury - assuming the CI passes on the latest commit, I'd say I'm done with this PR.

The core changes in this PR are:

  • renamed PyEQLEOS to Phreeqc2026EOS and the engine string from pyeql to phreeqc2026 as per your suggestion.
  • Our phreeqc compiled .so/.dll/.dylib stuff is now available as the pyEQL._phreeqc submodule, which in turn is used internally by the pure-python pyEQL.phreeqc submodule.
  • If we ever encounter a platform for which we're unable to compile phreeqc, we'd do, in our CI:
export PYEQL_ENABLE_EXT=OFF
pip install .

The same would be used by a user if they're on an exotic platform (and a regular pip install is failing for them):

export PYEQL_ENABLE_EXT=OFF
pip install pyeql

or simply:

PYEQL_ENABLE_EXT=OFF pip install pyeql
  • We have a utility variable pyeql.phreeqc.IS_AVAILABLE, which is guaranteed to work at all times under all platforms, but which may return True or False depending on whether the compiled extension is available or not. This is what we're using in the EOS class like this:
    def __init__(
       ...
        if not IS_AVAILABLE:
            raise RuntimeError("pyEQL phreeqc support is not available in this installation")

and in one of our phreeqc-engine-specific tests like this:

if not IS_AVAILABLE:
    pytest.skip(
        "pyEQL._phreeqc extension not available",
        allow_module_level=True,
    )

This same utility variable can be used in rest of the pyEQL codebase (and by its users) at other places as necessary.

  • Since we're back to a single-package situation (pyEQL), we can simply go back to whatever versioning system pyEQL was using before. I see that 1.3.2 is the latest version and this is obtained through the git tag using setuptools_scm, so that mechanism will still work (if it was working before).

As long as things look good, we should probably merge this PR and re-update the post-process CI trigger (triggering the CI on PR being closed, and re-enabling the release job) in a separate PR, so that the CI-specific stuff remains separate from any internal code changes.

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 ! Please make a note to capture the contents of your explanatory post in the documentation somewhere (in a future PR).

@rkingsbury rkingsbury added enhancement github_actions Pull requests that update GitHub Actions code labels Jan 23, 2026
@rkingsbury rkingsbury changed the title Trying to fix the postprocess workflow Fix the postprocess workflow; refactor PHREEQC2026 wrapper Jan 23, 2026
@rkingsbury rkingsbury merged commit a2a7833 into main Jan 23, 2026
46 of 47 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement github_actions Pull requests that update GitHub Actions code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants