learningequality / learningequality/kolibri
Enable ruff's SIM rules for resource handling and readability
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 1.1k
- Forks
- 972
- Avg merge
- 7d 16h
- Merged PRs (30d)
- 33
Description
❌ This issue is not open for contribution. Visit Contributing guidelines to learn about the contributing process and how to find suitable issues.
Overview
docs/code_quality.rst says whoever allocates a resource releases it, and SIM115 is the lint equivalent. The rest of SIM is readability cleanup that comes with it. Adds SIM with three rules excluded, 110 violations.
Complexity: Medium
Target branch: develop
Context
15 of the 110 have a safe fix, 63 more need --unsafe-fixes, and 32 are hand edits.
| Rule | Count | What it flags |
|---|---|---|
SIM102 collapsible-if |
27 | if a: wrapping only if b: |
SIM115 open-file-with-context-handler |
24 | open() outside a with block |
SIM118 in-dict-keys |
14 | k in d.keys() |
SIM103 needless-bool |
11 | if x: return True / return False |
SIM910 dict-get-with-none-default |
7 | d.get(k, None) |
SIM110 reimplemented-builtin |
6 | loop that reimplements any() or all() |
SIM300 yoda-conditions |
6 | if 5 == x |
SIM201 negate-equal-op |
5 | not a == b |
SIM101 duplicate-isinstance-call |
3 | |
SIM401 if-else-block-instead-of-dict-get |
3 |
SIM115 has no automatic fix: ruff will not restructure a bare open() into a with block, because it cannot tell where the handle's lifetime should end. All 24 are manual, and each changes when the file closes, so they are behaviour changes rather than cleanups.
SIM118's fix is marked unsafe because k in d.keys() and k in d differ if the object is not a real dict.
The Change
[tool.ruff.lint] select should gain SIM, with SIM105, SIM108 and SIM117 added to ignore, and the resulting violations cleared in the same PR.
Out of Scope
- Migrating any other resource type to a context manager. This sub-issue covers what
SIM115flags, which is file handles. - The other rule sets listed in the parent tracking issue.
Acceptance Criteria
-
[tool.ruff.lint] selectincludesSIM, andignoreincludesSIM105,SIM108andSIM117. -
uvx ruff check --select SIMreports no violations outside those three rules. - Every former
SIM115site closes its handle through awithblock, with no handle left open past the scope that created it. - No
# noqacomment in the diff names anySIMrule. -
pytest kolibri/passes. -
prek run --from-ref upstream/develop --to-ref HEADpasses.
AI usage
I used Claude Code to measure this rule set against develop and to draft this issue. The counts come from ruff check --statistics. Excluding SIM105, SIM108 and SIM117 is my call, on performance, Python version and readability grounds respectively.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with docs/code_quality.rst and the [tool.ruff.lint] configuration, then run uvx ruff check --select SIM to inspect the 110 violations. Clear the reported rules while preserving resource lifetimes, and verify the acceptance commands: pytest kolibri/ and prek run --from-ref upstream/develop --to-ref HEAD.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- tooling
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 35/100