-
Notifications
You must be signed in to change notification settings - Fork 2
Linting & formatting via ruff #209
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
Conversation
add rule to safe fixes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces ruff as a unified formatter and linter for the Python codebase, replacing the need for separate tools like black, isort, and Flake8. The changes include both formatting (quote style, line breaks, indentation) and automated linting fixes (removing unused imports, fixing logical operators).
Key changes:
- Addition of
pyproject.tomlconfiguration for ruff formatting and linting rules - Automated formatting applied across all Python files (quote style standardization, line breaks, indentation)
- Safe automated linting fixes applied (unused imports removed, logical operator corrections)
Reviewed Changes
Copilot reviewed 113 out of 115 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| TEKDB/pyproject.toml | Adds ruff configuration specifying double-quote style and enabling E711 safe fixes |
| docs/Data/models_new_fk.py | Reformatted Django model definitions with multi-line field declarations and double quotes |
| TEKDB/manage.py | Added noqa comment to suppress unused import warning |
| TEKDB/login/views.py | Reformatted with double quotes, removed unused imports (HttpResponse, logout) |
| TEKDB/login/urls.py | Standardized quote style to double quotes |
| TEKDB/login/models.py | Removed unused django.db.models import |
| TEKDB/login/admin.py | Removed unused django.contrib.admin import |
| TEKDB/explore/views.py | Extensive reformatting with double quotes, explicit imports replacing wildcards, fixed logical operators |
| TEKDB/explore/urls.py | Reformatted URL patterns with double quotes and improved line breaks |
| TEKDB/explore/tests/test_views.py | Removed unused import, reformatted with double quotes |
| TEKDB/explore/templatetags/explore_tags.py | Added noqa comment for function name redefinition, reformatted |
| TEKDB/explore/models.py | Reformatted model definitions with multi-line formatting |
| TEKDB/explore/migrations/*.py | Reformatted migration files with multi-line formatting and double quotes |
| TEKDB/explore/context_processors.py | Reformatted with double quotes, improved exception handling |
| TEKDB/explore/apps.py | Changed to double quotes |
| TEKDB/explore/admin.py | Replaced wildcard import with explicit PageContent import |
| TEKDB/configuration/ | Multiple files reformatted with double quotes and multi-line formatting |
| TEKDB/TEKDB/widgets.py | Removed unused imports, reformatted with double quotes |
| TEKDB/TEKDB/views.py | Replaced wildcard imports with explicit imports, reformatted, added noqa for bare except |
| TEKDB/TEKDB/urls.py | Reformatted URL patterns, removed unused imports |
| TEKDB/TEKDB/tests/ | Multiple test files reformatted, explicit imports replacing wildcards |
| TEKDB/TEKDB/migrations/*.py | Reformatted migration files with multi-line formatting |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
|
||
| else: | ||
| Raise("{} is an invalid hash function. Please Enter MD5 value") | ||
| raise ValueError("{} is an invalid hash function. Please Enter MD5 value") |
Copilot
AI
Oct 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error message format string is missing the hash_function argument. Should be: raise ValueError(f\"{hash_function} is an invalid hash function. Please Enter MD5 value\")
| raise ValueError("{} is an invalid hash function. Please Enter MD5 value") | |
| raise ValueError(f"{hash_function} is an invalid hash function. Please Enter MD5 value") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or ... MD5 Value".format(hash_function)
pollardld
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feels like an improvement. Thank you!
rhodges
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So many comments - thanks for your patience and perseverance. Everything looks good.
@rhodges thank you so much for the thorough review! |
asana task
This PR is a big change, but we do not need to adopt all of these changes. This PR is a bit on the ambitious side, and I would be happy to revert some changes if we want to be more cautious. I have applied both ruff formatting and linting in this change. The PR is probably best read commit by commit.
Why ruff:
I ended up choosing ruff as a formatter and linter because it includes the tools that I would have installed anyway (black, isort, Flake8) bundled in one. It is also very fast, and seems to be emerging as the python ecosystem standard (from what I can tell). I found the ability to specify the configuration pretty easy, so we can tailor the formatting and linting to our needs. It also integrates nicely with vscode with their extension.
Approach:
I started by installing
ruffwithpip install ruff, and applying formatting withruff format. Those changes are reflected in this commit. That commit also includes addingpyproject.tomlto include some configuration for formatting and linting.Once I felt okay about the formatting changes I checked to see how many linting issues would appear with running
ruff check. This resulted in ~500 errors, most of which seemed like relatively safe fixes. I first letruff check --fixapply safe fixes. These changes are captured in this commit. These changes mostly include removing unused imports and variables, and fixing a few logical order issues, ex:if not "PlaceID" in fields->if "PlaceID" not in fields.I then went through the different issues that were flagged and approached them one issue at a time. The different formatting codes are documented in the Flake8 Rules. I made changes to the ones that I thought were safe. For the changes I thought were not safe to do in this round, I added
# noqa: codecomments to suppress the errors. I think that it is worth making an tech debt task to revisit these comments, and perhaps we can do this on a quarterly or bi-annual basis as we continue developing in this repo.As of the latest commit, there are no linting errors and the tests pass. I could definitely do more manual testing to feel more secure in these changes, but overall I think these are lower risk, and we maybe even fixed a bug or two.
Questions
TODO:
Next steps
Since
rufffocuses on python formatting, we would need a separate formatter for the django templates. I'd like to do that in a separate PR since this is already a big change.Other features that we could include if we decided to adopt formatting and/or linting in the project: