Skip to content

Select DTZ and make every datetime explicitly UTC - #38626

Draft
antiguru wants to merge 6 commits into
MaterializeInc:mainfrom
antiguru:python-lint-rules
Draft

Select DTZ and make every datetime explicitly UTC#38626
antiguru wants to merge 6 commits into
MaterializeInc:mainfrom
antiguru:python-lint-rules

Conversation

@antiguru

@antiguru antiguru commented Sep 2, 2026

Copy link
Copy Markdown
Member

Stacked on #38625, which is stacked on #38624. Both must land first, so until they do the diff here also shows their commits. Review only the Select DTZ and make every datetime explicitly UTC commit.

This is the first of the additional lint rules discussed on #38625. The others are outlined at the bottom; they are not in this branch yet.

A naive datetime carries no offset, so what it means depends on the machine that produced it. In a codebase whose subject is timestamp semantics that is a poor default. The 54 sites this rule reports divide into two kinds.

The first kind is absolute time, and there the rule finds real defects. Epoch values are UTC by definition, so fromtimestamp without a timezone reinterprets them in local time, wrong by the machine's offset. The same applies to utcnow, which returns a naive value that then compares incorrectly against anything aware, to date.today, and to a strptime format ending in a literal Z that never records the zone it just parsed.

The second kind is deadline and elapsed-time arithmetic, end_time = now() + timedelta(...) followed by while now() < end_time. Both sides move together, so the offset cancels and reading the clock as UTC changes nothing. Worth saying plainly: those would be better served by time.monotonic, which no clock adjustment can move, and this change does not address that.

Why this needed manual work

Timezone-aware and naive values cannot be compared or subtracted, so a partial conversion raises TypeError at runtime, and neither ruff nor pyright would catch it. Values that flow into each other had to be converted together:

  • delete_after and _format_expires in scratch.py
  • get_last_modification_date against max_modification_date in data_io.py

Sources the rule does not police were audited as well, since a naive value can still arrive from fromisoformat or a bare constructor. canary-load already normalises its fromisoformat result before comparing, so nothing there conflicts.

Where a value is rendered rather than compared, output moves from local time to UTC. That affects a handful of log and assertion messages, and makes them unambiguous.

Verification

bin/lint passes in full, including pyright. Separately, all 26 touched files were import-loaded to catch anything the static checks miss, which matters because temporal.py had its date import replaced with datetime.

Remaining rules, not in this branch

Measured against the tree, with the caveats found while investigating:

  • B905, zip() without strict=, 44 sites. Direct precedent: clippy.toml bans std::iter::Iterator::zip with "use Itertools::zip_eq instead", so the project has already taken this position on the Rust side. It needs care rather than a bulk edit, because strict=True turns silent truncation into a runtime exception, and several sites may zip genuinely unequal sequences: zip(this_stats.keys(), other_stats.keys()) in parallel-benchmark, zip(self.params, args) in output_consistency/operation.py, and zip(*dependency_sets) in mzbuild.py.
  • B006, mutable argument defaults, 88 sites, for example environment_extra: list[str] = [] in checks/mzcompose_actions.py. Latent rather than currently firing, but one append turns any of them into state shared across calls.
  • B904, raise inside except without from, 55 sites, which discards the causing traceback.

Deliberately not proposed: S (3130 findings, bandit against test infrastructure), T (1838, bans print in CLI tools), FBT (1434), INP (474, all false positives since standalone mzcompose.py files correctly lack __init__.py).

One rule that looked promising and is not worth taking: B023, function uses loop variable, 13 sites. They are benign here. In mzbuild.py the closures go straight to spawn.run_with_retries, which invokes them inside the same iteration, so late binding never bites.

Release notes

No user-visible changes.

🤖 Posted by Claude Code

antiguru and others added 6 commits September 2, 2026 13:56
…eter

`bin/pyactivate` validates the interpreter running it and then hands
virtualenv creation to `uv venv` without naming an interpreter. `uv` resolves
one by its own preference order, which favors uv-managed installs over the
system Python, so the virtualenv can end up on a different and older Python
than the one the check just accepted. On a machine with system Python 3.14 and
a uv-managed 3.10 present, the virtualenv is built on 3.10. Passing
`sys.executable` removes that second, independent choice and makes the check
authoritative. It also aligns the two creation paths, since the `venv.create`
fallback right below already builds from the running interpreter.

That fallback passes `clear=True`, and `uv venv` is now given `--clear` to
match. Control reaches this branch only when the virtualenv is missing or its
Python will not execute, which is exactly when it should be replaced. Without
the flag `uv` refuses to touch an existing directory and aborts, so a
virtualenv left behind without its `dep_stamp` wedges every later invocation
instead of being rebuilt.

The floor moves from 3.10 to 3.13. Python 3.10 has been security-only for
years and reaches end of life in October 2026, and the tree had already
drifted past it: two mzcompose files import `LiteralString` from `typing`,
which needs 3.11. That went unnoticed because
`ci/test/lint-main/checks/check-python-version.sh` only byte-compiles the
tree, and compilation does not resolve imports, so a newer-than-floor import
passes the check and fails at run time. 3.13 is what the CI builder already
runs, since that is the system Python in Debian 13.

Raising `target-version` lets ruff replace `datetime.timezone.utc` with the
`datetime.UTC` alias and pull `LiteralString` back from `typing`, which is the
whole of the mechanical churn here. Ruff trails at `py312` because the pinned
version predates 3.13 and rejects `py313`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… bootstrap

The version gate in `main` inspects the interpreter that runs the script, while
the code the repository executes runs inside `misc/python/venv`. Those need not
agree, because the virtualenv outlives the interpreter it was built from, and
the reuse path only confirmed that a `dep_stamp` existed and that the
virtualenv's Python could execute at all. Raising the minimum therefore left
every existing virtualenv below it in place, and this branch is the first to
depend on that difference, since `datetime.UTC` does not exist before 3.11. The
liveness probe now also reports the version, so a virtualenv older than the
minimum takes the existing recreation path.

`uv venv --clear` refuses a target directory that is not a valid virtualenv and
suggests `--force`. That refusal is an error rather than the `FileNotFoundError`
the surrounding code catches, so it would abort with a traceback instead of
falling back to `venv.create`. The half-finished directory this leaves behind is
the case the comment above the check already describes, and `--force` is the
flag that actually matches `clear=True`.

The developer guide named 3.12 and derived it from the default Python in the
most recent Ubuntu LTS. That release is now 26.04 "Resolute Raccoon", which
ships 3.14, so the stated rule no longer produced the stated version and taken
literally would demand a version newer than the one CI runs. The guide now
gives the minimum directly and explains that it tracks the CI builder image,
noting that a current LTS satisfies it.

The minimum lives in `MIN_HEXVERSION` and `MIN_VERSION` so the gate, the probe,
and the operator-facing messages cannot drift apart.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`uv venv --force` was added in uv 0.11.17. Earlier versions exit with a usage
error, which reaches `subprocess.check_call` as a `CalledProcessError` and not
as the `FileNotFoundError` that selects the `venv.create` fallback, so
`bin/pyactivate` would abort. Everything in the tree runs through that script,
and the CI builder image installs uv 0.9.10, so the flag cannot be used here.

The version probe added alongside it makes the timing worse rather than
academic. Any developer holding a virtualenv below the new minimum takes the
recreation path on their first invocation, which is exactly the path that would
have failed, and deleting the virtualenv by hand leads to the same call.

Removing the directory before invoking `uv` behaves the same on every version
and needs no flag. It also settles what `--clear` and `--force` disagree about,
a path that exists but is not a virtualenv, which is the half-finished state the
comment above the check already describes. The `venv.create` fallback is
unaffected, since it creates a directory that does not exist.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The pinned ruff was 0.0.292, released in October 2023, and it rejects `py313`,
so the lint target trailed the minimum version the repository enforces. This
upgrade closes that gap and lets the `UP` rules see the language level we
actually run on.

Two interface changes come with it. Linting now requires the `check`
subcommand, since a bare `ruff <files>` is no longer accepted, which affects
`bin/fmt` and the lint check script. Top-level linter settings moved under a
`lint` section, so `select` and `isort` become `lint.select` and
`lint.isort`.

Retargeting to 3.13 surfaces rewrites the old target could not suggest. The
mechanical ones are PEP 695 type parameters in place of `TypeVar`, dropping
default type arguments, and a few string and annotation modernizations. The
`TypeVar` bindings that the PEP 695 conversion left with no remaining
references are removed, since ruff rewrites the signature but does not clean
up the declaration.

The two `mzexplore` enums become `StrEnum`. For `ExplainStage` nothing changes
at all, because it already defined `__str__` to return its value, and that
override is now redundant. For `ItemType`, `str()` and formatting now produce
the value rather than `ItemType.NAME`. No caller observes that: every use goes
through `sql()`, which reads the value either way, and formatting the member
directly does not appear anywhere.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Of 38 `noqa` directives, 29 had no effect. They were invisible because the rule
that reports them was not selected, and they predate the ruff upgrade: version
0.0.292 reports the same 29.

Most are star imports annotated `F401 F403`. A star import triggers F403 and
never F401, so only the F403 half ever did anything, and those directives are
narrowed to it. Three name rules from families this repository does not select,
`SLF001`, `E731` and `BLE001`, are removed along with one blanket directive on
an import that is used. The eight that remain sit on genuine re-exports in
`mzexplore/__init__.py`, where F401 does fire.

Selecting RUF100 keeps the set honest, since a directive that stops applying now
fails the lint rather than accumulating.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A naive datetime carries no offset, so what it means depends on the machine
that produced it. In a codebase whose subject is timestamp semantics that is a
poor default, and the 54 sites this rule reports divide into two kinds.

The first kind is absolute time, and there the rule finds real defects. Epoch
values are UTC by definition, so `fromtimestamp` without a timezone reinterprets
them in local time, which is wrong by the machine's offset. The same applies to
`utcnow`, which returns a naive value that then compares incorrectly against
anything aware, to `date.today`, and to a `strptime` format ending in a literal
`Z` that never records the zone it just parsed.

The second kind is deadline and elapsed-time arithmetic, `end_time = now() +
timedelta(...)` followed by `while now() < end_time`. Both sides move together,
so the offset cancels and reading the clock as UTC changes nothing. Worth
saying plainly: these would be better served by `time.monotonic`, which no
clock adjustment can move, and this change does not address that.

Timezone-aware and naive values cannot be compared or subtracted, so a partial
conversion raises `TypeError` at runtime and neither ruff nor pyright would
catch it. The pairs that flow into each other were converted together and
checked by hand: `delete_after` and `_format_expires` in `scratch.py`, and
`get_last_modification_date` against `max_modification_date` in `data_io.py`.
Sources the rule does not police were audited too, and `canary-load` already
normalises its `fromisoformat` result before comparing.

Where a value is rendered rather than compared, output changes from local time
to UTC. That affects a handful of log and assertion messages, and makes them
unambiguous.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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