Skip to content

Enable nine more previously-ignored ruff lint rules - #63

Merged
mmacferrin merged 1 commit into
mainfrom
ruff-unignore-rules-v2
Jul 28, 2026
Merged

Enable nine more previously-ignored ruff lint rules#63
mmacferrin merged 1 commit into
mainfrom
ruff-unignore-rules-v2

Conversation

@mmacferrin

Copy link
Copy Markdown
Collaborator

Second pass at trimming the [tool.ruff.lint] ignore list in pyproject.toml, following #62. This removes nine more rules and fixes the ten errors they raise. The ignore list goes from 130 to 121 entries.

As before, rules were chosen by running ruff check src --extend-select ALL --statistics to get the real violation count behind each ignored rule, then picking those whose fixes are mechanical and carry no behavior change.

Rules enabled

Rule Sites Change
C404 unnecessary-list-comprehension-dict 1 dict([(k, v), ...]) rewritten as a dict comprehension
PERF401 manual-list-comprehension 1 append loop replaced with a list comprehension
RET503 implicit-return 1 _fmt_size() restructured so its final return is reachable
RUF012 mutable-class-default 1 mutable class attribute annotated as ClassVar[dict[str, str]]
RUF013 implicit-optional 1 classes: list = None spelled as classes: list | None = None
RUF059 unused-unpacked-variable 1 unused unpacked base renamed to _base
SIM118 in-dict-keys 1 config section iterated directly rather than via .keys()
SIM212 if-expr-with-twisted-arms 1 None if not datadir else datadirdatadir or None
UP031 printf-string-formatting 2 percent-format strings replaced with f-strings

Notes

RET503_fmt_size() iterates ("B", "KB", "MB", "GB", "TB") and returns when nbytes < 1024 or unit == "TB", so the loop can never fall through and ruff's literal fix would have appended an unreachable return None. Instead the loop now covers ("B", "KB", "MB", "GB") with the TB case returned explicitly afterward. The two forms were compared across 11 magnitudes from 0 to petabyte-scale and produce identical output for every input.

RUF059 — the line base, ext = os.path.splitext(results_dataframe_file) appears three times in validate_dem.py (lines 967, 1327, 1731). The first two use base on the following line; only line 1731 leaves it unused, and only that one is renamed.

SIM212 — ruff's suggested fix is datadir if datadir else None, which would immediately violate FURB110 (enabled in #62). The or form satisfies both rules.

RUF012 — adds from typing import ClassVar to icesat2_database_v2.py. The annotation is purely declarative; the attribute remains an ordinary class-level dict at runtime.

B905 (zip-without-explicit-strict, 15 sites) is again deliberately left out. It is the one remaining low-count rule that is not mechanical: each call site needs a decision about whether the zipped iterables are guaranteed equal-length, and strict=True converts today's silent truncation into a ValueError. It deserves its own pass.

Verification

  • ruff check src, ruff format --check src, and prek run --all-files all pass.
  • ivert --help and ivert options run; all touched modules import cleanly.
  • Behavior spot-checks beyond the linter: _fmt_size old vs. new compared across 11 input magnitudes (no mismatches); list(section) == list(section.keys()) confirmed against the real config for the SIM118 change; SIM212 equivalence checked for "", None, a path, and "."; the ClassVar dict confirmed still readable with correct values; both rewritten ValueError messages confirmed to render as before.
  • Two modules could only be checked with py_compile, for environment reasons unrelated to this branch: validate_dem.py needs transformez, which is not installed locally, and plot_results_slope_centrality.py imports a nonexistent import_parent_dir module (pre-existing on main).

pytest is not installed in the local environment, so the test suite was not run.

Remaining work

Still one step toward #40, not the whole thing. The largest remaining block is the PTH* cluster (~380 hits across 20 rules), a genuine os.pathpathlib refactor rather than a sweep. Other sizable groups still ignored include ANN*, ERA001, E501, T201, and EM101/EM102.

Part of #40.

Remove C404, PERF401, RET503, RUF012, RUF013, RUF059, SIM118, SIM212 and
UP031 from the ruff lint ignore list and fix the ten resulting errors:

- C404: build a dict comprehension directly instead of dict([(k, v), ...]).
- PERF401: replace an append loop with a list comprehension.
- RET503: restructure _fmt_size() so the final return is reachable. The
  loop could never fall through, so ruff's literal fix would have added
  dead code; dropping 'TB' from the loop and returning it explicitly is
  equivalent for every input.
- RUF012: annotate a mutable class attribute as ClassVar.
- RUF013: spell an implicit Optional argument as 'list | None'.
- RUF059: rename an unused unpacked variable to _base. Two other call
  sites unpack the same expression but do use 'base'; only this one was
  flagged.
- SIM118: iterate a config section directly instead of via .keys().
- SIM212: rewrite 'None if not datadir else datadir' as 'datadir or None'.
  Ruff suggests the ternary form, but that would trip FURB110.
- UP031: replace two percent-format strings with f-strings.

Part of #40.
@mmacferrin
mmacferrin merged commit 5096b95 into main Jul 28, 2026
3 checks passed
@mmacferrin
mmacferrin deleted the ruff-unignore-rules-v2 branch July 28, 2026 22:38
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