Skip to content

Conversation

@jsstevenson
Copy link
Contributor

@jsstevenson jsstevenson commented Sep 2, 2025

close #56

  • Add some more Ruff lint rules.
  • Misc config cleanup

A few notes

  • Previous PR was from before the repo overhaul, I just went ahead and made a new one rather than trying to handle merge weirdness
  • pin exact ruff version. Unfortunately as far as I know, the pre-commit hook locks you into a specific ruff version JUST for your precommit hook, so using a more relaxed constraint on the base dependency declaration risks drift between the two
  • bump up the ruff version a bit, there's an autofix in 0.12.11 for this one logging rule that is really annoying to fix manually
  • bump up precommit constraint and check it on the precommit side too. Might as well.
  • ignore dirs like docs/ (I think running linting and formatting on the Sphinx conf.py file is often more trouble than it's worth, for example) and bin/sbin (I'm usually hesitant to touch these when doing style commits in the biocommons repos because they don't have unit tests and might have specific uses I'm not privy to)
  • Unfortunately toml-sort kinda messes up a lot of ways I'd prefer to be marking up the rule configs, maybe there's a way to suppress this but for now I just left it sparse
  • No need to specify target-version, ruff infers it from the requires-python value (which btw needs to be fixed). A couple of the format settings were just set to their default value.
  • Remove the EM lint ruleset because @theferrit32 hates it

style: update ruff formatting and lint configs
@reece
Copy link
Member

reece commented Sep 2, 2025

@jsstevenson Great updates. Thanks.

It appears to me that this change activates a bunch of new ruff tests. I'm concerned that tightening the screws too much right now will make it hard to adopt in repos because everything will fail out of the gate. (I realize that they won't be tested until they're changed.) Any comments on that concern?

@jsstevenson
Copy link
Contributor Author

It appears to me that this change activates a bunch of new ruff tests. I'm concerned that tightening the screws too much right now will make it hard to adopt in repos because everything will fail out of the gate. (I realize that they won't be tested until they're changed.) Any comments on that concern?

Totally. For context, my approach with style changes has been to take a rough guess at an ideal set of configs, take a run at implementing them in some core repos (hgvs, seqrepo, bioutils, anyvar), and then update the template proposal as I encounter lint rules that seem suboptimal or unnecessary. For seqrepo, bioutils, and anyvar, the PRs are fully complaint with the rules in this PR and all tests are passing (with the caveat that pyproject.toml will need some restructuring because toml-sort broke the way I was annotating the configs). HGVS is, of course, its own beast, and I set aside a few major lint groups as needing independent PRs to achieve compliance (PTH and ANN being big ones) (I also still need to circle back on the HGVS PR, it has a test failing so it's still in draft).

To take bioutils as an example: biocommons/bioutils#78. The majority of changes in that PR come from either precommit (e.g. trailing whitespace) or formatting/autofixing. There were a dozen or so cases of function type annotations to add, but I think we'd want to be checking in CI for adherence in any new code committed to the repo, so I felt okay about taking the time to bring the existing code into compliance.

All of that being said, I'd be ok handling this PR and the downstream changes in individual repos myself after the core template is implemented. I think you'll find that PTH, SIM, PT, and especially LOG, which are already included in main, will encompass the vast majority of manual changes (though there's an autofix in ruff 0.12.11 that should handle the most common LOG violation).

@jsstevenson jsstevenson closed this Sep 8, 2025
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.

Update linting/formatting/etc

4 participants