Apply ruff formatting and linting along with relevant refactoring #371
Conversation
kenibrewer
force-pushed
the
refactor/code-quality-updates
branch
from
d85ae2a to
90e47f7
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #371 +/- ##
==========================================
- Coverage 95.01% 94.96% -0.05%
==========================================
Files 56 56
Lines 3187 3139 -48
==========================================
- Hits 3028 2981 -47
+ Misses 159 158 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Nice job! Looks like a lot of good changes here to help simplify things. I left a few comments for your consideration as part of this review.
Additional mentions:
- I noticed that the Codecov.io comment mentioned something about a potential missing file. Is this something which could be addressed before a merge?
- Consider updating the PR title to reflect the specific changes applied (i.e.
ruff-formataddition, applied formatting, etc).
There was a problem hiding this comment.
@d33bs Incorporated your feedback and made revisions to style guide section.
There was a problem hiding this comment.
Thanks for addressing the comments @kenibrewer ! I'm going to give this my approval and would like to ask for a further review from @gwaybio to make sure we aren't missing something especially when it comes to the test changes. I'd still recommend changing the title of the PR to be more descriptive to the type of changes occurring - adding specificity if possible.
kenibrewer
changed the title
This change makes the dev dependency group optional so it won't be installed by default with `poetry install` unless included by `--with dev`. This brings this group in alignment with the docs group.
ruff-format is a rust implementation of the black formatter that is 10x faster with 99.9% compatiblity. This commit swaps out black for the ruff-format as the pre-commit hook that checks formatting.
pyupgrade checks for (and automatically fixes) code patterns from older python versions with the updated patterns. This commit turns on that check and applies the fixes to the codebase. These fix for our codebase consist mostly of replacing `"a{}c".format('b')` with `f"a{'b'}c".
To support a large collection of additional ruff checks that are applied to the codebase, this commit adds the ruff vscode extension to the devcontainer configuration. This will support developers in providing in-line highlighting and automatic fixing.
This applies some ruff native quality checks to the codebase. The two changes applied to our codebase are iliminating the use of an implicit Optional variable type and using *-expansion to generate combined lists instead of concatenation.
This applies pycodestyle E and W checks to the codebase. The most common change from these checks is replacing "!= None" with "is not None"
This commit applies flake8-simplify checks to the codebase. The non-style refactoring included in this commit is primarily condensing nested ifs and using or/and instead.
No additional changes required for codebase
This commit applies pyflakes checks to the codebase and removes a large number of unused variables and imports. Notable non-automatic fixes in this commit include removing variables that are never checked while testing error conditions. This also removes an unused invocation of consensus during the test_aggregate set of tests that appears to be left over from copy and pasting tests.
This commit applies the flake8 builtins checks that checks for variables shadowing python built-in functions and variables. As part of this, we rename the docs variable `copyright` to the alias `project_copyright` to avoid shadowing the python built-in function. https://www.sphinx-doc.org/en/master/usage/configuration.html#confval-project_copyright
In preparation for turning on the flake8-bandit checks (which will need to be a separate PR), this adds the most important per-file ignore statement to allow the use of assert statements during tests, but keep them forbidden within the codebase itself.
kenibrewer
force-pushed
the
refactor/code-quality-updates
branch
from
19bbdd7 to
7d74b35
Compare
There was a problem hiding this comment.
I incorporated some more of @d33bs feedback in the docs and added an additional clarifying assert for the cell_loc tests.
I'll move ahead and merge these commits after tests finish running.
Description
Code quality update adopting the use of ruff format instead black, and ruff check for a suite of linting checks.
This PR includes extended comments in the commits which are broken down using a commit for each new set of checks and the appropriate fixes. It is recommended to review the PR using this commit by commit approach.
Additional checks/fixes planned for the future are represented by commented out lines in pyproject.toml. I attempted to apply each of them but they represent more substantial changes that should be reviewed separately.
What is the nature of your change?
Checklist
Please ensure that all boxes are checked before indicating that a pull request is ready for review.
📚 Documentation preview 📚: https://pycytominer--371.org.readthedocs.build/en/371/