Skip to content

Conversation

@eriknw
Copy link
Contributor

@eriknw eriknw commented Jul 23, 2025

I added some pre-commit hooks to get us closer to releasing (and for general quality assurance).

I may have gone a little overboard.

@seberg, @Schefflera-Arboricola, please don't be afraid to make changes to these pre-commit hooks and configurations, especially config for ruff. My rationale is that it's easier to begin with linting tools for a new project than it is to add linting rules to old projects, so might as well try many to see what's useful. Any others you'd like to try (or avoid)?

I made changes incrementally so that each addition may be checked commit by commit.

I also ran sp-repo-review to find recommendations and looked at https://github.com/scientific-python/cookie (but not exhaustively).

@lucascolley
Copy link
Contributor

(comment from the sidelines, feel free to ignore!)


are you planning to support uv? If so, I highly recommend lefthook instead of pre-commit, as it's less complicated to let uv manage the environments rather than pre-commit setting up its own (also faster + nicer UI IMO). Example from another new repo: https://github.com/data-apis/array-api-typing/blob/main/lefthook.yml.

It also plays nicely with Pixi, which may be of interest for other reasons: https://github.com/prefix-dev/pixi/blob/main/lefthook.yaml.

@eriknw
Copy link
Contributor Author

eriknw commented Jul 24, 2025

Thanks for chiming in @lucascolley! Comments from the sidelines are very welcome, and I'm hoping we can eventually get spatch nice enough to use in scipy.

are you planning to support uv?

What do you mean by "support uv"? Using uv/uvx should generally just work, right, since it can get info from pyproject.toml? Should we have a uv.lock file? When should they be updated? Or do you mean to use uv in e.g. github actions and contributor documentation? I think we're likely to add noxfile.py as is done in https://github.com/scientific-python/cookie, which uses uv.

I'm not an expert in uv or pixi, so happy to take suggestions and learn best practices.

This is also the first I've heard of lefthook. Looks pretty cool! It'd be okay with me if anybody feels compelled to use it instead of pre-commit, but I'm not likely to change it in the near future.

@lucascolley
Copy link
Contributor

Yes, I think shipping a uv.lock file was what I had in mind.

I'm not sure what the best practice is for uv lockfile updates, but for Pixi we can use renovate: https://github.com/data-apis/array-api-extra/blob/main/renovate.json.

Copy link
Contributor

@seberg seberg left a comment

Choose a reason for hiding this comment

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

Thanks for all of this cleanup! Two small things that don't matter, let's get it in :).

https://github.com/MarcoGorelli/auto-walrus

"You are a monster!", and the style checks even hate my two spaces after a full stop ;)!1!

(To be clear, I am happy with just using these strict style fixes, it's not like I have to agree to prefer every change :).)

eriknw and others added 5 commits July 24, 2025 10:24
Co-authored-by: Sebastian Berg <sebastian@sipsolutions.net>
Co-authored-by: Sebastian Berg <sebastian@sipsolutions.net>
Co-authored-by: Sebastian Berg <sebastian@sipsolutions.net>
@eriknw
Copy link
Contributor Author

eriknw commented Jul 24, 2025

Thanks @seberg! I got your suggestions in. I also don't mind having two spaces after a period like you like to do (even though that's so 10 years ago :-P) and am generally not a stickler for formatting, but I do like using tools to automate things even when I don't 100% agree with 'em. Just wait until auto-walrus does its thing on your code!

Also, fyi my preference for max line length is between 88 and 100 characters. Thanks all for being flexible and wanting to move forward. If anybody has a work setup where they prefer shorter line lengths, we can adjust it.

Thanks for clarifying @lucascolley. Let's get uv.lock done in a new PR.

Squash merging.

@eriknw eriknw merged commit 5f81cc1 into scientific-python:main Jul 24, 2025
10 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.

3 participants