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

Feature/update pre commit #169

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Conversation

TommyMatthews
Copy link
Contributor

Updates pre commit to use the latest version of ruff and not black.

Had to add a few things to ignore in ruff.toml, generally I used the project template approach for guidance. The two I'm least sure about were the following (done in order to avoid having to manually edit loads of files):

  • E501 line length - apparently 88 is the line length preferred by black but with this set it was suggesting editing a lot of lines in supposedly black approved code
  • E721 - forces isinstance() over is type() etc

Fixed ruff's complaint about using int | float instead of just float. Done in a specific revert so easy to revert if we don't like this.

@TommyMatthews TommyMatthews marked this pull request as ready for review January 25, 2024 15:59
@davidhopkinson26
Copy link
Contributor

on the E501 override - did you take a look at what changes the formatter would make here? Keeping line length down is one of the features of Black I would quite like to preserve here. So long as code functionality is not affected (which it shouldn't be!) I would be in favour of just making the automatic changes suggested by the formatter now.

@TommyMatthews
Copy link
Contributor Author

@davidhopkinson26 there are about 800 lines flagged by this error, but they are left unchanged by the formatting and the linting - I don't think that this functionality has made it into ruff formatting yet. Perhaps we leave this PR out until it's in?

@davidhopkinson26
Copy link
Contributor

@davidhopkinson26 there are about 800 lines flagged by this error, but they are left unchanged by the formatting and the linting - I don't think that this functionality has made it into ruff formatting yet. Perhaps we leave this PR out until it's in?

@TommyMatthews I think another option might be to set the line length parameter in ruff.toml to a slightly higher number to accommodate the code as is. docs. Would suggest trying 88 first as I'm guessing ruff might be set lower by default to be getting this issue.

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