Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

migrate to ruff and uv #1986

Open
banteg opened this issue Apr 3, 2024 · 9 comments
Open

migrate to ruff and uv #1986

banteg opened this issue Apr 3, 2024 · 9 comments

Comments

@banteg
Copy link
Contributor

banteg commented Apr 3, 2024

Elevator pitch:

this would massively speed up the build process

Value:

test runners and devs get almost instant installation, format and lint times

Dependencies:

make sure it works with setuptools_scm

Design approach:

Task list:

  • remove black, isort, flake8 from dev deps
  • add ruff to dev deps
  • bootstrap installation with uv instead of pip

Estimated completion date:

Design review:

Do not signoff unless:

    1. agreed the tasks and design approach will achieve acceptance, and
    1. the work can be completed by one person within the SLA.
      Design reviewers should consider simpler approaches to achieve goals.

(Please leave a comment to sign off)

Copy link

linear bot commented Apr 3, 2024

@fubuloubu
Copy link
Member

fubuloubu commented Apr 3, 2024

uv pip now supports setup.py workflow, so it should be trivial to update the CI to support as a first step. There is no need to migrate to a pyproject.toml workflow yet to support the migration

Migrating to ruff would be slightly more involved/impactful for slightly less benefit (linting is not long pole for CI, but local dev would be a lot faster)

Should be a separate issue to migrate to ruff

@Aviksaikat
Copy link
Contributor

Afaik ruff configs can be done using ruff.toml so there's no need to unnecessarily populate pyproject.toml.

@banteg
Copy link
Contributor Author

banteg commented Apr 19, 2024

the only thing i had to do is to add this to pyproject.toml, the default config seems compatible with ape's black/isort/linting rules. we can always enable more useful rules later.

[tool.ruff]
line-length = 100

@fubuloubu
Copy link
Member

the only thing i had to do is to add this to pyproject.toml, the default config seems compatible with ape's black/isort/linting rules. we can always enable more useful rules later.

[tool.ruff]
line-length = 100

I guess we can just matrix the CI against both sets of tools for now

@fubuloubu
Copy link
Member

As far as I see it:

  • installing and caching using uv in the CI should be completely transparent from whatever a dev uses to work locally, so it is safe to upgrade the CI today
  • we have noticed some deficiencies in ape currently when pip is not installed, but think those have been fixed. At the very least, we should test it is independent of the existence of pip
  • if ruff mirrors our current settings for the linting tools it replaces, then I still think we should test against those tools in the short term as not everyone has upgraded, and we should find the discrepancies
  • AFAIK, ruff doesn't have an mdformat like tool, and also we should make sure some of the additional flake8 extensions we use are configurable in ruff

@Aviksaikat
Copy link
Contributor

Aviksaikat commented Jul 19, 2024

Here I have tried to configure ruff similarly to how ape is doing with black & isort https:/Aviksaikat/ape-utils/blob/main/pyproject.toml

Here is an example use of ruff along with mdformat

lint-check = [
  "echo \"ruff VERSION: `ruff --version`\"",
  "ruff format .",
  "ruff check . --fix",
  "mypy src/ape_utils/",
  "echo \"mdformat VERSION: `mdformat --version`\"",
  "mdformat README.md docs/examples.md",
  "echo \"check VERSION: `check --version`\"",
  "cz check --rev-range $(git rev-list --all --reverse | head -1)..HEAD"
]

NOTE: I'm using hatch for this project. Currently hatch can't pass the --pre flag to uv (pypa/hatch#1617) which creates issues with pre-released dependencies. We can still use uv with rye

We can easily use pip along with hatch to run in CI as well like this

on: ["push", "pull_request"]

name: Test

concurrency:
  # Cancel older, in-progress jobs from the same PR, same workflow.
  # use run_id if the job is triggered by a push to ensure
  # push-triggered jobs to not get canceled.
  group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }}
  cancel-in-progress: true

jobs:
 tests:
   runs-on: ${{ matrix.os }}

   strategy:
     matrix:
       os: [ubuntu-latest, macos-latest]
       # * Don't need to run on all versions as hatch matrix will do the job
       python-version: ["3.9"]
   steps:
    - uses: actions/checkout@v4
    - name: Set up Python
      uses: actions/setup-python@v5
      with:
        python-version: ${{ matrix.python_version }}
    - name: Install dependencies
      run: |
        python -m pip install --upgrade pip
        pip install hatch pre-commit
        hatch env create
    - name: Lint and typecheck
      run: |
        hatch run lint:lint-check
    - name: Run Tests
      run: |
        hatch run test:test

@wakamex
Copy link
Contributor

wakamex commented Jul 25, 2024

it's been uv installable since May #2000 (comment)

only ruff is left, and possibly using uv in ci, which has had an attempt recently #2165

@fubuloubu
Copy link
Member

it's been uv installable since May #2000 (comment)

only ruff is left, and possibly using uv in ci, which has had an attempt recently #2165

In my own personal testing I've noticed that uv has difficulty installing feature branch github repos correctly.

That may be okay for our use overall, but a limitation on what can be done for local development (and potentially limiting ourselves on how next release feature branches work)

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

No branches or pull requests

4 participants