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

Enable all pycodestyle and isort rules in Ruff and fix violations #1463

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

matt-graham
Copy link
Collaborator

@matt-graham matt-graham commented Sep 23, 2024

Updates Ruff configuration to enable all pycodestyle error (E prefix), pycodestyle warning (W prefix) and isort (I prefix) rules. Previously we used the default Ruff configuration which only selects a subset of the E rules (E4, E7 and E9), no W rules and doesn't enable any isort rules (but we separately used isort package to check import ordering).

The only violations of the additional pycodestyle error rules now enabled were of the E501 lines-too-long rule, and this PR also fixes all instances of this rule being violated in src/tlo and tests, and adds a global directory level ignore for src/scripts (under the assumption we probably don't care too much about formatting violations in the scripts).

The only violations of the additional pycodestyle warning rules enabled were of W291 trailing-whitespace and W293 blank-line-with-whitespace rules, with the this PR also fixing all instances of this rule being violated.

Ruff's isort rules are very close to the rules checked by isort itself and the only violation after enabling isort rules was due to presence of a commented out import in src/scripts/hiv/projections_jan2023/analysis_full_model.py. The main advantages of using Ruff rules rather than separate isort are a slight improvement to run time of checks by not requiring separate installation and running of isort, and more usefully, for those who use a Ruff integration in their IDE, unsorted imports will now be flagged directly in IDE. I am still commonly caught out by the isort check failing on pushing regularly, so this should hopefully make this less likely by making it easier to catch violations earlier!

While this PR touches a lot of files, all of the changes should have no effect on actual code as they are just changes to whitespace / line reflowing (hiding whitespace changes in diff should make it a bit easier to review!).

@matt-graham matt-graham requested a review from tamuri September 23, 2024 14:56
@matt-graham matt-graham force-pushed the mmg/stricter-ruff-rule-selection branch from db883e1 to e1b1ef8 Compare September 24, 2024 12:15
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@tdm32 Have you got any active branches that update src/tlo/methods/hiv.py that the below changes (reformatting lines to make them conform with 120 character line length limit) might conflict with? If so we can hold off merging this until those changes are in to master and then update here.

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.

1 participant