Migrate Python environment management from conda to uv#1082
Merged
Conversation
Replace the four platform-specific conda environment files with a single uv-managed project defined in pyproject.toml plus a committed uv.lock, and pin the interpreter via .python-version (3.12.9). - pyproject.toml: declare runtime dependencies under [project], build/test tooling (PyInstaller, pytest) under a dev dependency group, and carry the Intel-macOS ruamel.yaml.clib build workaround as a uv constraint. The former conda-forge binary packages (numpy, lxml, pyedflib, pytables) now install as PyPI wheels, which are available for all supported platforms. - Constrain spikeinterface to 0.103.0 so tutorial data generation keeps the 0.5 Hz LFP bandpass filter it relies on. - CI/deploy workflows: replace setup-miniconda and conda env caching with astral-sh/setup-uv and `uv sync --locked`, and add the resulting virtual environment to PATH. Node.js continues to be provisioned separately. - developer_guide.rst: document the uv-based setup and the Node.js prerequisite that conda previously provided. Validated on macOS arm64: uv sync, the pyflask test suite, the PyInstaller build, and booting the bundled executable (all 72 neuroconv interfaces load). Windows, Linux, and Intel-macOS builds are validated by CI. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
# Conflicts: # environments/environment-Linux.yml # environments/environment-MAC-apple-silicon.yml # environments/environment-MAC-intel.yml # environments/environment-Windows.yml
The project is a uv virtual project that is never built or published, so the version field only duplicated package.json and could drift. Declare it dynamic so package.json remains the single source of truth. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Tolerate matplotlib already being absent (|| true) so the step cannot fail a run if a future resolution drops it, and remove matplotlib in the macOS and Windows deploy builds as well so the released distributable matches the one exercised by the build tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adding the [project] table to pyproject.toml makes the build:flask step emit src/nwb_guide.egg-info metadata. Ignore it so it is not committed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The workflows ran steps in a login shell (bash -l), a leftover from conda activation. On macOS the login shell invokes path_helper, which rebuilds PATH from /etc/paths.d and moves the system framework Python ahead of the injected .venv/bin. Bare `python` then resolved to framework Python 3.14 instead of the project's 3.12 venv, so `python -m PyInstaller` and the spawned Flask backend failed with "No module named PyInstaller" / "No module named 'flask'" (macOS BuildTests, E2ELiveServices, and ExampleDataTests). The GitHub `bash` shell runs with --noprofile, so path_helper never runs and the venv stays first. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Temporary diagnostic: the E2E suite intermittently fails on the macOS x64 runner when the backend hot-reload watcher restarts Python mid-test. Log the path of each change event so CI reveals which file under src/pyflask triggers the restart. To be reverted once identified. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This reverts commit e5a5734.
rly
marked this pull request as ready for review
July 15, 2026 15:59
Add `cache: 'npm'` to every setup-node step so `npm ci` restores its downloads from cache instead of re-fetching them each run (the dominant CI cost, ~4 min on the macOS x64 runner). Keep `npm ci --verbose` for debugging; the caching, not the flag, is what speeds it up. Wrap only the macos-15-intel E2E run in nick-fields/retry (3 attempts): that runner intermittently crashes Chromium/Electron subprocesses, which is a pre-existing, dependency-independent flake. The action is pinned to the v4.0.0 commit since it is a third-party action. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
pauladkisson
left a comment
pauladkisson
left a comment
Collaborator
There was a problem hiding this comment.
Looks good, just some clarifying comments.
| # network-service), which fails the E2E run. Retry only on that runner. | ||
| - if: matrix.os == 'macos-15-intel' | ||
| name: Run tests (retried on the flaky x64 runner) | ||
| uses: nick-fields/retry@ad984534de44a9489a53aefd81eb77f87c70dc60 # v4.0.0 |
Collaborator
There was a problem hiding this comment.
What does this job do?
Collaborator
There was a problem hiding this comment.
And also, why can't you just @v4?
Collaborator
Author
There was a problem hiding this comment.
The job retries the below command (npm run coverage:tutorial) if it fails, with 5 max attempts and a timeout of 25 minutes, in case it gets stuck.
Recent best practices for github actions, especially non-official actions, are to pin a specific commit instead of the mutable version tag.
Last year, a popular GitHub Action tj-actions/changed-files was compromised and most release tags were updated to point to a new commit with code that leaked secrets. https://www.stepsecurity.io/blog/harden-runner-detection-tj-actions-changed-files-action-is-compromised.
To prevent these attacks, GitHub now supports "immutable releases", but adoption has been slow across the actions community. Surprisingly, not even the github-maintained core "actions/checkout" action is immutable: actions/checkout#2316 . The setup-uv action has started to make immutable releases, and so pinning to a version tag there is fine (note that they stopped publishing "floating tags" (v4, v4.0) for similar reasons). Until actions are immutable, pinning to a specific release commit gives the highest security possible, at the costs of the workflow being less readable and the action not auto-updated. I think in a year, actions will be immutable, and we can return to tag-based pointers.
I did not update the action pins in all the workflows in this PR, just the changed ones. The others ones will be updated in a separate PR.
Collaborator
There was a problem hiding this comment.
Huh, good to know. Thanks for the info!
Replace the floating astral-sh/setup-uv@v6 major tag with the exact v8.3.2 release across all seven workflows. v8.0.0 was the first setup-uv release published under GitHub's immutable releases, so this tag cannot be repointed at new code the way a major tag can. Upstream stopped publishing major and minor tags at v8 for that reason, citing the tj-actions compromise, so a full version is the only available pin. None of the v7 or v8 breaking changes reach these workflows: the removed server-url input and the reworked manifest-file format are both unused, the enable-cache input is unchanged, and the Node 24 runtime is satisfied by the GitHub-hosted runners every job already uses. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Record the reason the removal step exists, established in #906: GUIDE never imports matplotlib, it arrives only as a transitive dependency, and its hidden libraries break the PyInstaller build when it is present in the environment. Point at `uv tree --invert --package matplotlib` rather than naming the current dependency paths. The suspected source in #906 was spikeinterface[qualitymetrics] via neuroconv[testing]; today it resolves through ndx-miniscope, mearec, and meautility instead, so a hardcoded list would go stale again. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
pauladkisson
approved these changes
Jul 16, 2026
Collaborator
Author
|
Thanks for the review @pauladkisson ! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replaces conda with uv for the Python environment. The four
environments/environment-*.ymlfiles become a single uv project (pyproject.toml+ committeduv.lock), with the interpreter pinned via.python-version(3.12.9). The conda-forge binaries (numpy,lxml,pyedflib,pytables) now install as PyPI wheels, which exist for all supported platforms.Changes
pyproject.toml: runtime deps under[project]; build/test tooling (PyInstaller, pytest) in adevgroup; versiondynamicsopackage.jsonstays authoritative. Transitive workarounds as uvconstraint-dependencies:spikeinterface >= 0.104.8,scipy >= 1.18.0,pandas < 3(mirroring Fix daily CI failures from spikeinterface/scipy dependency drift #1080's env-file floors), andruamel.yaml.clib != 0.2.13(Intel-mac build).pandas < 3avoids a read-onlyDataFrame.valuescrash in spikeinterface's Phy extractor (upstream read_phy / PhySortingExtractor raises "assignment destination is read-only" under pandas >= 3.0 SpikeInterface/spikeinterface#4687).setup-miniconda->astral-sh/setup-uv+uv sync --locked+.venvonPATH; run shellbash -l {0}->bash(login shell ran macOSpath_helper, which shadowed the venv'spython);npmdownloads cached viacache: 'npm';matplotlibremoval made resilient and applied in deploy builds too. Node.js still viaactions/setup-node.docs/developer_guide.rst: uv-based setup + Node.js prerequisite..gitignore:.venv/,*.egg-info/.environments/environment-*.ymlfiles.Intel E2E flakiness
E2ELiveServices / macos-15-intelis intermittently red due to Chromium/Electron subprocess crashes on that runner (pre-existing, fails on the conda #1080 branch too; not a migration regression). Mitigated by wrapping only that runner's E2E innick-fields/retry(5 attempts), pinned to the v4.0.0 commit.Validation
Verified on macOS arm64:
uv sync --locked, pyflask tests (5/5),npm run build:flask, and the bundled executable boots and loads all 72 neuroconv interfaces. Windows, Linux, and Intel-macOS are validated by CI.Notes for reviewers
flask-corsunified to4.0.0(Windows conda env used3.0.10, an apparent accidental divergence).uv.lock; a follow-up can deliberately bump the stack.🤖 Generated with Claude Code