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

Pre-commit tools and black update #4655

Merged
merged 5 commits into from
Mar 14, 2024

Conversation

peterjc
Copy link
Member

@peterjc peterjc commented Mar 7, 2024

  • I hereby agree to dual licence this and any previous contributions under both
    the Biopython License Agreement AND the BSD 3-Clause License.

  • I have read the CONTRIBUTING.rst file, have run pre-commit
    locally, and understand that continuous integration checks will be used to
    confirm the Biopython unit tests and style checks pass with these changes.

  • I have added my name to the alphabetical contributors listings in the files
    NEWS.rst and CONTRIB.rst as part of this pull request, am listed
    already, or do not wish to be listed. (This acknowledgement is optional.)

Some minor housekeeping with the style checking.

This might get fixed in flake8, but we can probably disable
this in favour of black style anyway.
Reverted unwanted changes (mostly arrays in tests, but also
some whitespace in doctests).

Remaining changes are standardising spacing between module
docstring and imports, and lower-case \x<hex> in strings.
@peterjc
Copy link
Member Author

peterjc commented Mar 7, 2024

Cross reference PyCQA/flake8#1922 for E704 false positives [fixed link]

We may want to move the ruff linter configuration into its own
file soon...
@JoaoRodrigues
Copy link
Member

JoaoRodrigues commented Mar 10, 2024

Not necessarily related to this PR, but should we have a look at all the linters/rules we use and maybe standardize them? Right now in our CI workflows I see:

  • Our CI runs pre-commit
  • We have a separate pre-commit.ci service

In the CI workflow, we have:

  • flake8
  • black
  • ruff
  • etc.

Could we maybe consider dropping black and flake for ruff alone? Will that make a big difference in style, whereas it might simplify our CI quite a bit?

For instance, these specific flake8 plugins have native support in ruff:

We can also replace black by ruff, since it does the same formatting job.

The only thing we cannot use ruff for are the docs/rst.

@peterjc
Copy link
Member Author

peterjc commented Mar 10, 2024

I've been moving some my own stuff to ruff in favour of flake8 and black, but it doesn't cover everything we're using. Dropping pydocstyle on this PR is a step in that direction, happy to take that further 👍

@JoaoRodrigues
Copy link
Member

👍 👍 Are there any rules you're following when dropping the linters? Like, run with ruff only, then run the old linters to see if it missed anything?

@peterjc
Copy link
Member Author

peterjc commented Mar 10, 2024

You have to enable non-default ruff checks, which ought to be fine if we're already using the original it was based on (e.g. flake8-bugbear), but occasionally there are differences.

I'm hoping once ruff adds array formatting we can use it in place off black.

@peterjc
Copy link
Member Author

peterjc commented Mar 13, 2024

@JoaoRodrigues was that an implicit PR approval?

@JoaoRodrigues
Copy link
Member

Yes! Sorry, I didn't think you'd be waiting for my approval!

@peterjc peterjc merged commit 79dbcf7 into biopython:master Mar 14, 2024
35 checks passed
@peterjc peterjc deleted the pre-commit-and-black-update branch March 14, 2024 11:05
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.

None yet

2 participants