Skip to content

Updated actions to use uv#1248

Merged
VisLab merged 8 commits intohed-standard:mainfrom
VisLab:fix_extras
Feb 28, 2026
Merged

Updated actions to use uv#1248
VisLab merged 8 commits intohed-standard:mainfrom
VisLab:fix_extras

Conversation

@VisLab
Copy link
Member

@VisLab VisLab commented Feb 28, 2026

No description provided.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates GitHub Actions workflows to install Python tooling and project dependencies via uv (using astral-sh/setup-uv) instead of actions/setup-python + pip, aiming to simplify setup and leverage uv caching.

Changes:

  • Replaced actions/setup-python + pip install steps with astral-sh/setup-uv + uv pip install across CI, lint/format, docs, and notebook workflows.
  • Enabled uv caching (enable-cache: true) in the updated workflows.
  • Simplified/removed explicit pip caching and pip upgrade steps where uv is now used.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
.github/workflows/test_installer.yaml Switches installer test workflow to create a venv with uv and install the package using uv pip.
.github/workflows/spec_tests.yaml Uses uv to install test dependencies before running spec tests.
.github/workflows/ruff.yaml Installs Ruff via uv and runs lint checks.
.github/workflows/notebook_tests.yaml Installs example dependencies via uv before running notebook-related tests.
.github/workflows/mdformat.yaml Installs mdformat tooling via uv and runs formatting checks.
.github/workflows/links.yaml Installs docs dependencies via uv before building docs and running link checking.
.github/workflows/docs.yaml Uses uv for docs dependencies and removes separate pip cache step.
.github/workflows/codespell.yaml Installs codespell dependencies via uv and runs spelling checks.
.github/workflows/ci_windows.yaml Uses uv to install test dependencies on Windows CI.
.github/workflows/ci_cov.yaml Uses uv to install coverage/lint/test deps before coverage workflow steps.
.github/workflows/ci.yaml Uses uv to install docs/test dependencies in the main CI workflow.
.github/workflows/black.yaml Installs Black via uv and runs formatting checks.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@github-actions
Copy link

PR Review: Updated actions to use uv

This PR cleanly migrates all 12 CI workflows from actions/setup-python + pip to astral-sh/setup-uv + uv pip. The approach is consistent and the structural changes are correct. A few items worth addressing:

Important

Wrong base branch: Per project conventions (CLAUDE.md / .rules/git.md), PRs should target develop, not main. This PR is currently targeting main directly.

Suggestion

test_installer.yaml line 32: $GITHUB_WORKSPACE is unquoted in the shell command:

uv pip install $GITHUB_WORKSPACE

Should be:

uv pip install "$GITHUB_WORKSPACE"

This is a shell best-practice — workspace paths rarely have spaces in GitHub Actions, but quoting is cheap insurance.

Positive notes

  • Replacing the manual actions/cache@v5 + pip cache with enable-cache: true on setup-uv is the correct approach — uv manages its own cache internally.
  • The test_installer.yaml rewrite also silently fixes a pre-existing bug: the old step wrote WORKDIR to $GITHUB_OUTPUT (step outputs), but then referenced it as $WORKDIR (shell variable) in later steps — which would always be empty. The new relative-path approach (workdir/.venv) is cleaner and correct.
  • uv pip install --system is appropriate for all the single-environment workflows. The isolated-venv pattern in test_installer.yaml (create venv → activate → install) is also correct.
  • Python matrix versions including 3.14 will be handled cleanly by uv's built-in Python management, which is a genuine improvement over setup-python.

@github-actions
Copy link

PR Review: Updated actions to use uv

This PR consistently migrates all 12 CI workflows from actions/setup-python + pip to astral-sh/setup-uv + uv pip. The changes are clean and well-structured.

Summary

No critical or blocking issues found.

Positive changes

  • Bug fix in test_installer.yaml: The original code wrote WORKDIR to $GITHUB_OUTPUT and then referenced it as $WORKDIR in subsequent steps — a bug since GITHUB_OUTPUT values are step outputs (accessible via steps.<id>.outputs), not environment variables. The new approach using relative paths (workdir/.venv) is correct.

  • Intentional UV_SYSTEM_PYTHON omission in test_installer.yaml: Correct. This workflow creates and activates its own venv, so uv detects VIRTUAL_ENV and uses it automatically. The other workflows without explicit venvs correctly set UV_SYSTEM_PYTHON: "1".

  • Cache consolidation: Replacing the manual actions/cache@v5 pip cache steps with enable-cache: true in setup-uv is correct and idiomatic — uv's built-in cache is more efficient.

  • Consistent migration: All 12 workflows updated uniformly.

Suggestion

test_installer.yaml: uv pip install $GITHUB_WORKSPACE — the variable should be quoted to guard against paths with spaces: uv pip install "$GITHUB_WORKSPACE".

@github-actions
Copy link

The PR is a clean, consistent migration of all 12 CI workflows from actions/setup-python + pip to astral-sh/setup-uv@v5 + uv pip. The approach is correct and follows the recommended pattern for using uv in CI.

What is done well:

  • Consistent use of UV_SYSTEM_PYTHON="1" across all workflows that install to the system Python, replacing the old pip install --upgrade pip bootstrapping.
  • enable-cache: true on setup-uv correctly replaces the manual actions/cache steps previously used for pip/virtualenv caching.
  • test_installer.yaml correctly omits UV_SYSTEM_PYTHON since it creates and activates an explicit venv — uv respects VIRTUAL_ENV when set.
  • The old test_installer.yaml had a latent bug where WORKDIR was written to $GITHUB_OUTPUT (step output) but referenced as $WORKDIR (shell env var) in subsequent steps. Those two are not the same thing. The new relative-path approach silently fixes this.
  • docs.yaml step indentation is normalized to be consistent with the rest of the file.

Suggestion:
test_installer.yaml line 27 — uv venv workdir/.venv does not pin the Python version. setup-uv@v5 with python-version sets a default, but if uv has multiple Python versions in its cache the resolution can be ambiguous. Adding --python makes the intent explicit and matches the matrix version:

run: uv venv workdir/.venv --python ${{ matrix.python-version }}

No critical or important issues found.

@github-actions
Copy link

PR Review: Updated actions to use uv

This PR consistently migrates all 12 CI workflows from actions/setup-python + pip to astral-sh/setup-uv@v5 + uv pip. The approach is correct and well-executed. No critical or important issues found.

What is done correctly

  • UV_SYSTEM_PYTHON: "1" is set at the job level on every workflow that installs into the runner's system Python, and correctly omitted from test_installer.yaml, which creates and activates an explicit venv (uv respects VIRTUAL_ENV automatically).
  • Cache consolidation: replacing the manual actions/cache@v5 + pip-cache steps with enable-cache: true on setup-uv is idiomatic and correct — uv manages its own cache internally.
  • test_installer.yaml bug fix: the original wrote WORKDIR to $GITHUB_OUTPUT (step output slot) but then referenced it as $WORKDIR (shell variable) in subsequent steps. Those are not the same thing, so the original was broken. The new relative-path approach (workdir/.venv) silently fixes this.
  • $GITHUB_WORKSPACE is quoted (uv pip install "$GITHUB_WORKSPACE") — correct shell hygiene.
  • Python version is pinned in the venv creation step (uv venv workdir/.venv --python <version>) — explicit and correct.
  • docs.yaml indentation is normalized to be consistent with the rest of the file.
  • CLAUDE.md change (develop to main) matches the actual repo default branch and the target of this PR.

Minor note

ci_cov.yaml combines ruff and editable package installs into a single command (uv pip install ruff -e .[docs,test]). This is valid uv syntax and resolves all dependencies together in one pass, which is preferable to two sequential pip calls.

@github-actions
Copy link

PR Review: Updated actions to use uv

Overall: The migration from setup-python + pip to astral-sh/setup-uv is clean and correct. uv's built-in caching (enable-cache: true) replaces the manual actions/cache@v5 steps. The test_installer.yaml also fixes a pre-existing bug where the old code wrote WORKDIR to $GITHUB_OUTPUT but then referenced it as a plain env var ($WORKDIR), which never worked.

Two suggestions below:

Suggestion 1: Inconsistent venv approaches across workflows

Four workflows (black.yaml, codespell.yaml, mdformat.yaml, ruff.yaml) use the --python .venv/bin/python flag and uv run --python .venv/bin/python for every command. The other eight workflows add .venv/bin to $GITHUB_PATH and call commands directly. Both work, but the inconsistency makes the codebase harder to maintain.

The $GITHUB_PATH approach is simpler and avoids repeating --python .venv/bin/python on every command. Consider standardizing the four outlier workflows to match the pattern used in ci.yaml, ci_cov.yaml, etc.

Suggestion 2: Unquoted extras specifiers

Lines like uv pip install -e .[docs,test] pass .[docs,test] unquoted to bash. In bash, [docs,test] is a bracket expression glob that matches single characters (d, o, c, etc.). It passes through literally because no matching files exist today, but this is fragile. Prefer -e ".[docs,test]".

@github-actions
Copy link

PR Review: Updated actions to use uv

Overall this is a good migration from setup-python/pip to uv. The direction is correct. A few issues need attention before merge.


Important: uv venv called with path duplicated

In 9 out of 12 changed workflows the venv creation step reads:

run: uv venv .venv --clear .venv

The positional PATH argument .venv appears twice. uv venv accepts a single positional path; --clear (if it is a valid boolean flag) takes no argument. Most CLI frameworks raise Got unexpected extra argument (.venv) when a positional is supplied twice.

The correct forms are:

# Option A – just (re)create, uv handles existing venvs gracefully
run: uv venv .venv

# Option B – clear before recreating (if --clear is a supported boolean flag)
run: uv venv .venv --clear

Affects: black.yaml, ci.yaml, ci_cov.yaml, codespell.yaml, docs.yaml, links.yaml, mdformat.yaml, notebook_tests.yaml, ruff.yaml, spec_tests.yaml.

Note that test_installer.yaml already uses the correct form (uv venv workdir/.venv --python ...).


Important: Inconsistent uv invocation patterns

The 12 workflows now use two different patterns:

Pattern A (black, ruff, codespell, mdformat):

- run: uv venv .venv ...
- run: uv pip install --python .venv/bin/python <pkg>
- run: uv run --python .venv/bin/python <cmd>

Pattern B (ci, ci_cov, docs, links, notebook_tests, spec_tests):

- run: |
    uv venv .venv ...
    echo "$GITHUB_WORKSPACE/.venv/bin" >> $GITHUB_PATH
- run: uv pip install -e .[...]    # no --python flag
- run: python -m ...               # plain command from PATH

Both work, but mixing them across the same repo is confusing and makes maintenance harder. Pattern B (PATH manipulation) is simpler and already used by the majority of workflows. The four linting-only workflows (black, ruff, codespell, mdformat) should be updated to match.


Suggestion: ci_windows.yaml – implicit PowerShell dependency

The "Create virtual environment" step uses PowerShell-specific syntax (Add-Content, $env:GITHUB_PATH) with no explicit shell: declaration:

run: |
  uv venv .venv --clear .venv
  Add-Content $env:GITHUB_PATH "$env:GITHUB_WORKSPACE\.venv\Scripts"

This works today because GitHub-hosted Windows runners default to pwsh, but an explicit declaration prevents silent breakage if the default ever changes:

run: |
  uv venv .venv
  Add-Content $env:GITHUB_PATH "$env:GITHUB_WORKSPACE\.venv\Scripts"
shell: pwsh

Minor: trailing whitespace

ci.yaml line 65 and spec_tests.yaml have trailing spaces after the pipe character on run: | . Not functional, but ruff/CI will flag it.

@github-actions
Copy link

PR Review: Updated actions to use uv

The migration from setup-python + pip to astral-sh/setup-uv is the right direction. uv provides faster installs and built-in caching. The overall structure is sound. A few issues need attention before merging.

Important

ci_windows.yaml: Implicit PowerShell dependency without shell: pwsh

The Create virtual environment step uses Add-Content $env:GITHUB_PATH (PowerShell-only syntax) in a run block with no explicit shell declaration. Windows runners default to pwsh, so this works today, but it creates a silent fragility: if defaults: run: shell: bash is ever added at the workflow or job level, this step breaks without a clear error. Annotate it explicitly with shell: pwsh on the step.

Suggestions

Inconsistent venv management across workflows

Four linting workflows (black.yaml, codespell.yaml, mdformat.yaml, ruff.yaml) use uv pip install --python .venv/bin/python and uv run --python .venv/bin/python, while the other eight add .venv/bin to GITHUB_PATH and invoke commands directly. Both approaches are correct, but the inconsistency adds cognitive overhead when maintaining the workflows. Consider standardising on the GITHUB_PATH pattern used by ci.yaml, ci_cov.yaml, etc.

ruff.yaml: missing trailing newline at end of file

The final line ruff check . has no trailing newline (flagged by No newline at end of file in the diff). Add a newline for POSIX compliance and clean diffs.

CLAUDE.md fix

The correction from develop to main as the PR target branch is correct.

@github-actions
Copy link

PR Review: Updated actions to use uv

Overall: The migration is well-structured and consistent across all 12 workflows. No critical or important issues found.

Positive Changes

  • enable-cache: true in setup-uv is a clean replacement for the manual actions/cache@v5 steps — uv's cache is more granular and efficient.
  • pyproject.toml regex fixes are correct: \.status properly escapes the dot (was matching any char + "status"), and (\./)?hed/_version\.py handles both relative and absolute path forms.
  • test_installer.yaml is simplified and fixes a pre-existing bug where $WORKDIR was written to $GITHUB_OUTPUT but referenced as a bare shell variable (step outputs are not shell variables).
  • docs.yaml indentation is now consistent with the other workflows.

Suggestions (minor)

ci_cov.yaml — combined install line: uv pip install ruff -e ".[docs,test]" is valid uv/pip syntax and works correctly, but separating these into two invocations (uv pip install ruff then uv pip install -e ".[docs,test]") would match the original two-step approach and be clearer.

test_installer.yaml — redundant --python flag: uv venv workdir/.venv --python ${{ matrix.python-version }} is redundant because setup-uv already sets UV_PYTHON to the matrix version. The other workflows omit this flag and rely on UV_PYTHON. Minor inconsistency, harmless.

uv venv --clear flag: Recreates the venv if it already exists. On fresh CI runners this is a no-op. Consistent usage across all workflows is fine.

@VisLab VisLab merged commit 8c007fe into hed-standard:main Feb 28, 2026
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants