The generated ruff config selects DTZ and ANN and then ignores both families #7

Activity

1. AlexeyShalaev commented on Sep 7, 2026

   

   AlexeyShalaev

   on Sep 7, 2026

   MemberAuthor

   More actions

   Confirmed, and worse than dead weight in one direction: I generated a project from the template today and dropped this into the package —

   def stamp():
       return datetime.datetime.now()
   
   
   def utc_stamp():
       return datetime.datetime.utcnow()
   
   
   def from_epoch(seconds):
       return datetime.datetime.fromtimestamp(seconds)

   make check says All checks passed!. The same file under --select DTZ,ANN --ignore ANN401,ANN204 reports seven findings (DTZ005, DTZ003, DTZ006, ANN202 ×3, ANN001). So the select list is promising three timezone rules that never run.

   What the libraries actually say

   Before deciding anything I ran the candidate rule sets over deadline-budget, clientwright, servicewright and sqlalchemy-foundation-kit (ruff 0.16.6, --isolated, read-only, nothing committed anywhere):

   DTZ: zero findings. Library code, tests, scripts/, examples/ — all four repositories, clean. And it is a real pass rather than a vacuous one: the code that touches time is already correct. clientwright/core/engine/base.py:32 computes (when - datetime.datetime.now(datetime.UTC)).total_seconds(), and sqlalchemy_foundation_kit/base/models.py:30 maps datetime.datetime to TIMESTAMP(timezone=True). So turning DTZ on costs nothing today and starts guarding the thing you named — deadlines, retention windows, TTLs. No follow-up issues to open.

   ANN: zero new findings in library code, in all four. mypy's disallow_untyped_defs really is doing that work. Which means ANN earns its keep exactly where mypy does not look — files = ["<package>"], so tests and scripts/ — and that is where I want it on rather than off.

   S101 is the only entry with a real cost. Moving it out of the global ignore newly flags five asserts, all in clientwright library code, all type narrowing rather than validation:

   clientwright/adapters/aiohttp/middleware.py:97   assert isinstance(response, aiohttp.ClientResponse)
   clientwright/adapters/requests/adapter.py:90     assert isinstance(response, requests.Response)
   clientwright/adapters/urllib3/adapter.py:93      assert isinstance(response, urllib3.HTTPResponse)
   clientwright/core/engine/aio.py:73               assert response is not None
   clientwright/core/engine/sync.py:72              assert response is not None
   

   That is a decision for that repository when it takes the update — a # noqa: S101 on each, or a raise — not something to pre-empt here.

   What I am going to ship

   select unchanged. ignore keeps only the rules this house genuinely disagrees with, and the entries that exist because of tests move to where they are true:

   [tool.ruff.lint]
   select = [...unchanged...]
   ignore = ["TRY003", "ANN401", "ANN204", "RUF012", "S104", "N802", "PERF401", "SIM105", "S607"]
   
   [tool.ruff.lint.per-file-ignores]
   "tests/**/*.py" = ["S101", "S105", "S106"]

   ANN401 and ANN204 stay and now mean something. S104 stays global — servicewright binds 0.0.0.0 in library code in four places, so scoping it to tests would be a lie in the other direction. TRY003, RUF012, N802, PERF401, SIM105 all fire in library code too; global is where they belong.

   What I rejected

   Dropping DTZ and ANN from select instead. Honest, and it throws away the one family nothing else covers. The point of the report is that this org computes everything from timestamps.

   Ignoring ANN in tests/, as the issue suggests. I measured it rather than assuming: enforcing it costs 15 findings across the four repositories — 1 in clientwright, 1 in servicewright, 13 in sqlalchemy-foundation-kit (six ANN001 on an async_session_factory argument, seven ANN202 on inner helpers). CONTRIBUTING.md already tells contributors "type hints on all functions and methods, including tests", and mypy never looks at tests/, so ruff is the only thing that can hold that line. Fifteen annotations is a fair price for the config and the contributing guide finally agreeing.

   A regression test, not just a fixed line. The failure mode here is a config that claims a rule it does not run, and nothing catches that. The template's CI gets a step that renders the project, feeds it a naive datetime.now() and an unannotated function, and fails if ruff stays quiet — plus an assert in tests/ that has to stay accepted, so the per-file-ignores block is checked too.

   One correction to the report: pg-partsmith has not diverged on this — it carries the same ignore line verbatim. Its only difference is extend-exclude = ["examples", "*.md"].
2. AlexeyShalaev commented on Sep 7, 2026

   

   AlexeyShalaev

   on Sep 7, 2026

   MemberAuthor

   More actions

   Correction to the list above — I transcribed the third one from the wrong line. clientwright/adapters/urllib3/adapter.py:93 is assert isinstance(view, Urllib3RequestView), narrowing the request rather than the response. Same shape, same conclusion.
3. AlexeyShalaev commented on Sep 7, 2026

   

   AlexeyShalaev

   on Sep 7, 2026

   MemberAuthor

   More actions

   Shipped in #8, CI green.

   The generated [tool.ruff.lint] now reads:

   select = [...unchanged...]
   ignore = ["TRY003", "ANN401", "RUF012", "S104", "ANN204", "N802", "PERF401", "SIM105", "S607"]
   
   [tool.ruff.lint.per-file-ignores]
   "tests/**/*.py" = ["S101", "S105", "S106"]

   Your file, run through the regenerated project — the same uv run ruff check . that make check calls — now reports all seven: DTZ005, DTZ003, DTZ006, three ANN202 and one ANN001. Before the change it printed All checks passed!.

   On the two families: DTZ is on rather than out of select, for the reason you gave. ANN is on as well, and I did not put it in the tests block. mypy reads <package>/ and never tests/, CONTRIBUTING.md already asks for "type hints on all functions and methods, including tests", and the measured cost of holding that line is fifteen annotations across three repositories — one in clientwright, one in servicewright, thirteen in sqlalchemy-foundation-kit. Config and contributing guide now say the same thing.

   S101, S105 and S106 moved into the tests block rather than staying global, which is the part that makes library code stricter than it was. S104 stayed global — servicewright binds 0.0.0.0 in library code in four places, so scoping that one to tests would have been the same kind of untruth pointing the other way.

   Nothing to file against the libraries. DTZ reports zero findings in deadline-budget, clientwright, servicewright and sqlalchemy-foundation-kit — library code, tests, scripts/ and examples/ alike. The code that touches time is already right, so no follow-up issues. The only thing an existing library owes the new config is the five narrowing asserts in clientwright and those fifteen test annotations, which each repo picks up whenever it takes the update.

   And the check that was missing: CI now renders the project, hands it a naive datetime.now() and an unannotated def, and fails if ruff stays quiet — plus a bare assert in tests/ that has to stay accepted, so the per-file-ignores block is covered too. Against the old config that step fails on the first probe; a family cannot be selected and ignored again without CI noticing.
4. AlexeyShalaev

   closed this as completedin #8on Sep 7, 2026