Use Pixi in macOS-intel CI - #13704
Conversation
|
Hey @scott-huberty, looking at the current diff I noticed you're setting up Found when switching to mne-python/.github/workflows/tests.yml Lines 157 to 165 in 8dd298f Wondered if a similar thing could work for |
| run: | | ||
| pixi run python tools/write-pixi-toml.py | ||
| pixi lock --no-install --manifest-path tools/ci/macos-intel/ | ||
| if: steps.status.outputs.dirty == 'true' |
There was a problem hiding this comment.
In this PR the steps are:
- run
spec_zero_update_versions.py(which may changepyproject.toml) - run
tools/sync_dependencies.py(updates README) - create lockfile for old CI and check its pins
- check for dirty files
- install pixi (if dirty)
- update the pixi.toml file for macos-intel CI (if dirty)
- run pre-commit (if dirty); specifically for
tools/hooks/update_environment_file.py - create PR (if dirty)
I'm wondering if we should instead:
- check for dirty repo immediately after step 1
- gate steps 2 & 3 on (if dirty)
- (optional) use a faster way of checking for dirty files, namely:
if $(git diff --quiet HEAD -- pyproject.toml); then
echo "dirty=true" >> $GITHUB_OUTPUT
fi@scott-huberty @larsoner @tsbinns WDYT?
There was a problem hiding this comment.
There could be the case where tools/sync_dependencies.py creates a diff independently from spec_zero_update_versions.py if a change was made in a PR to a dep specifier in pyproject.toml, but the README wasn't updated at the same time.
Currently, the missing update of the README would get caught and fixed the next time the SPEC0 action gets run, but the suggested change would gate this behind spec_zero_update_versions.py introducing some changes.
This could lead to deps listed in README being out-of-date for several weeks/months, which was what we tried to resolve in #13832 by adding the README syncing to the SPEC0 action.
Maybe it is safer therefore to only gate step 3 on whether it is dirty?
There was a problem hiding this comment.
ok right, thanks for the reminder. so we do want step 2 to always run. And it seems like the same logic could also apply to step 3? E.g., someone changes a minimum pin manually in a PR but then spec_zero_update_versions.py happens not to bump anything, then pyproject.toml wouldn't show up as dirty when this action runs (but we'd still want to update the old CI pins). This makes me think that we should not even gate steps 5-7 either; they should all just run and we should only use the dirty check to decide whether the action needs to make a commit.
notes:
- keeping the gating for step 7 is probably fine, since currently it's run via pre-commit, so
environment.ymldoesn't risk getting out of sync the same way that the README does... but I'd actually lean toward removing theenvironment.ymlupdater from pre-commit, and standardizing our procedure as "all of the trickle-down effects of dep/pin changes inpyproject.tomlhappen at the same time (weekly cron job)" instead of some-but-not-all being part of our pre-commit checks. - if we do switch to the faster way of checking for dirty files (and I'd like us to, because the faster way also avoids false positives when testing locally in the presence of untracked files), it shouldn't only look at
pyproject.tomlanymore, it should just be:git diff --quiet HEAD
There was a problem hiding this comment.
OK, all tests are passing!
So we should we remove the dirty gate from the install pixi / update pixi.toml steps (steps 5-6)?
|
TODO @scott-huberty make it so that the matrix python version specified in tests.yml gets inserted into the pixi.toml file |
- The Pixi job replaces teh MacOS pip job on main. - The MacOS pip job on main does not set MNE_TEST_ALLOW_SKIP - mne/conftest.py:1357 treats unset MNE_TEST_ALLOW_SKIP as “allow all skips"
scott-huberty
left a comment
There was a problem hiding this comment.
Doh... Never submitted this review, which was just 1 comment!!
| run: | | ||
| pixi run python tools/write-pixi-toml.py | ||
| pixi lock --no-install --manifest-path tools/ci/macos-intel/ | ||
| if: steps.status.outputs.dirty == 'true' |
There was a problem hiding this comment.
OK, all tests are passing!
So we should we remove the dirty gate from the install pixi / update pixi.toml steps (steps 5-6)?
|
after adding a "this is autogenerated" comment to the top of |
done in e5bd471 |
|
possible problem: seems like environment is getting MNE stable release (not main/PR): https://github.com/mne-tools/mne-python/actions/runs/30474035062/job/90651542841?pr=13704#step:18:61 probably because of this: https://github.com/mne-tools/mne-python/actions/runs/30474035062/job/90651542841?pr=13704#step:9:372 |
hmm The pixi.lock file has: EDIT: Maybe because |
I'm exploring the feasibility of using Pixi to resolve the virtual environment and run tests specifically for the MacOS Intel CI, which currently uses Mamba to create the virtual environment.
My initial strategy was to just to do
pixi init --import environment.yml, so that we can leverage our already defined YAML config file to pull in all the necessary dependencies, as well as configuringprefix-dev/setup-pixito automatically activate our virtual environment, so that we can run commands likepytestetc without having to prependpixi runeverywhere for the pixi CI...Unfortunately the only way I could get this to work was to disable use of a login shell.. Which of course makes all the condo-like CI's fail!!