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

Use uv for linter workflow #39701

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

Conversation

tobiasdiez
Copy link
Contributor

Currently, the linter workflow always uses the latest release of all dependencies. Thus, updates to these linters may result in broken CI (just happened yesterday see #39699).

Here, we use uv to lock the dependency of all tools used in the linter workflow. The lock file will be automatically updated using renovate.

Also remove pycodestyle as by now it can be replaced by ruff.

(The lock file also contains all sage dependencies and thus may be used to replace the manual updates of python deps in sage-the-distro @dimpase)

📝 Checklist

  • The title is concise and informative.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation and checked the documentation preview.

⌛ Dependencies

@tobiasdiez tobiasdiez requested a review from fchapoton March 14, 2025 14:51
@dimpase
Copy link
Member

dimpase commented Mar 14, 2025

this needs a bit of documentation. How was the lock file generated in the 1st place. How to update it manually.
I only see maintainLockFilesMonthly automation - how is it triggered, and where?

Copy link
Member

@dimpase dimpase left a comment

Choose a reason for hiding this comment

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

few sentences in developer docs, or just as comments, please. It's too cryptic now.

Copy link

github-actions bot commented Mar 14, 2025

Documentation preview for this PR (built with commit e563fe4; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

@tobiasdiez
Copy link
Contributor Author

You can manually update all locked packages using uv lock --upgrade or a single package via uv lock --upgrade-package xyz, see https://docs.astral.sh/uv/concepts/projects/sync/#upgrading-locked-package-versions.

The uv docs are very good, so I'm hesitant to put something in sage's dev docs, which then needs to be maintained and may get outdated relatively soon (given that uv is moving pretty fast at the moment).

The auto-update is done via the github app https://github.com/apps/renovate (needs to be installed by an admin, but that's pretty straigtforward). It will then open PRs with the upgraded lock file on a regular interval. An example of such a PR would be JabRef/JabRefOnline#2598.

@dimpase
Copy link
Member

dimpase commented Mar 14, 2025

Does uv support dealing with package hashes - assuming we want checking them for security purposes ?

@tobiasdiez
Copy link
Contributor Author

tobiasdiez commented Mar 15, 2025

Does uv support dealing with package hashes - assuming we want checking them for security purposes ?

Yes, here is an excerpt from the lock file, which contains the hashes as you can see:

[[package]]
name = "alabaster"
version = "1.0.0"
source = { registry = "https://pypi.org/simple" }
sdist = { url = "https://files.pythonhosted.org/packages/a6/f8/d9c74d0daf3f742840fd818d69cfae176fa332022fd44e3469487d5a9420/alabaster-1.0.0.tar.gz", hash = "sha256:c00dca57bca26fa62a6d7d0a9fcce65f3e026e9bfe33e9c538fd3fbb2144fd9e", size = 24210 }
wheels = [
    { url = "https://files.pythonhosted.org/packages/7e/b3/6b4067be973ae96ba0d615946e314c5ae35f9f993eca561b356540bb0c2b/alabaster-1.0.0-py3-none-any.whl", hash = "sha256:fc6786402dc3fcb2de3cabd5fe455a2db534b371124f1f21de8731783dec828b", size = 13929 },
]

If you want to use this info in sage-the-distro, I can see two paths: a) parse the uv lock file as part of bootstrap and put the info into the currently existing files under build or b) completely replace all python package handling in sage-the-distro by running something like uv sync --frozen to install all necessary python packages as specified in the uv lock file.

@dimpase
Copy link
Member

dimpase commented Mar 15, 2025

How about putting the following either in docs or in comments somewhere?

You can manually update all locked packages using `uv lock --upgrade` or a single package via `uv lock --upgrade-package xyz`, 
see https://docs.astral.sh/uv/concepts/projects/sync/#upgrading-locked-package-versions.

The auto-update is done via the github app https://github.com/apps/renovate (needs to be installed by an admin). 
It will then open PRs with the upgraded lock file on a regular interval. 

once the PR in question is open, it should be explicitly mentioned.

@user202729
Copy link
Contributor

What's the difference between uv and tox?

(also aren't we already using something… (conda?) that support installing exact version of packages anyway? Is it worth introducing another dependency for something that seems like a relatively infrequent thing?)

@dimpase
Copy link
Member

dimpase commented Mar 16, 2025

uv is 100 times faster, something like this. due to being written in rust.

it's increasingly being used instead of pip (also slow), as it can do what it does (and more)

@tobiasdiez
Copy link
Contributor Author

I've now added a bit of docs around uv, to be extended once we use it in more places.

--

What's the difference between uv and tox?

On very abstract terms, uv is a package manager and tox a task manager. But uv actually plans to become a universal tool that covers the whole dev process (eg tasks is a planned feature, so is conda support etc).

(also aren't we already using something… (conda?) that support installing exact version of packages anyway? Is it worth introducing another dependency for something that seems like a relatively infrequent thing?)

Sadly not all of these linters are available on conda, like relint. Also I'm planning to use uv for "meson + system dependency" config that doesn't use conda (since many sage devs are not the biggest fan of conda).

Copy link
Member

@dimpase dimpase left a comment

Choose a reason for hiding this comment

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

OK. Please make it Positive review.

@tobiasdiez
Copy link
Contributor Author

Thanks!

@sagemath/core please install the Renovate app, either by accepting the request I just send or by going to https://github.com/apps/renovate and clicking "Install". Thank you in advance!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants