Skip to content

fix(playground): resolve the 11 ruff violations in backend_manager - #89

Closed
basil-k-aji-dev wants to merge 1 commit into
google:mainfrom
basil-k-aji-dev:fix/playground-ruff-violations
Closed

basil-k-aji-dev wants to merge 1 commit into
google:mainfrom
basil-k-aji-dev:fix/playground-ruff-violations

Conversation

@basil-k-aji-dev

Copy link
Copy Markdown

Closes #29.

Problem

uv run ruff check . has failed on main since the playground merge (#24), with 11 errors in playground/backend_manager. The practical cost is the one #18 describes: every new pull request opens with a red Lint Python step that has nothing to do with its own contents.

$ uv run ruff check .
Found 11 errors.

Change

The fixes ruff already proposes:

Rule Sites Change
UP017 6 timezone.utc → UTC
UP045 2 Optional[X] → X | None
UP037 2 unquote the annotation

UTC is timezone.utc is True (the alias has existed since 3.11), so timestamp behaviour is byte-for-byte unchanged:

$ uv run python -c "from datetime import UTC, timezone; print(UTC is timezone.utc)"
True

On the two UP037 sites — worth a look, because they are the risky-looking ones

Both annotate an optional dependency, which is presumably why they were quoted in the first place:

try:
    import docker
    DOCKER_AVAILABLE = True
except ImportError:
    DOCKER_AVAILABLE = False
...
    self._client: Optional["docker.DockerClient"] = None

Removing the quotes is safe here because both are attribute annotations inside __init__. Python does not evaluate annotations on complex targets in a function body, so docker is never looked up when the package is absent. I checked rather than assumed, on the project's 3.12:

$ # with `docker` unimportable
self.x inside __init__ : OK (annotation not evaluated)
class-level attribute  : NameError: name 'docker' is not defined

So the identical annotation moved to class level would break the no-docker install path. It is not moving, and python -m compileall is clean on both packages.

Orphaned imports

The rewrites left timezone unused in three modules and Optional unused in two. F401 is in this repo's ruff ignore list so nothing flags them, but they are dead, so I removed them. bigquery_service keeps its pre-existing unused from datetime import datetime, timezone — that one predates this change and is outside the issue's scope.

Verification

Command Before After
uv run ruff check . Found 11 errors All checks passed
uv run ruff format --check . 616 files already formatted 616 files already formatted
uv run python scripts/quality_ratchet.py passed passed, counts unchanged
uv run python -m compileall on both packages — exit 0

uv run pytest -q is unaffected: playground/ is not in testpaths, and the full suite shows the same results as main on this host.

make typecheck could not be run in my environment — the pyright wheel downloads a Node runtime on first use and my sandbox blocks it. No touched file is in pyright-core.json.

`uv run ruff check .` has failed on main since the playground merge, so every
pull request opens with a red Lint step that has nothing to do with its own
contents.

Apply the fixes ruff already proposes: `datetime.UTC` for `timezone.utc`
(UP017), `X | None` for `Optional[X]` (UP045), and unquoted annotations
(UP037). `UTC is timezone.utc`, so the timestamp behaviour is unchanged.

The two UP037 sites annotate optional dependencies (`bigquery`, `docker`,
both imported inside `try/except ImportError`), which is presumably why the
annotations were quoted. Removing the quotes is safe here because both are
attribute annotations inside `__init__`: Python does not evaluate annotations
on complex targets in a function body, so the name is never looked up when the
package is absent. The same annotation at class level would raise NameError.

Drop the imports these rewrites orphaned (`timezone` in three modules,
`Optional` in two). F401 is disabled for this repo so ruff does not flag them,
but they are dead. `bigquery_service` keeps its pre-existing unused
`datetime, timezone` import, which predates this change.

Closes google#29.
@basil-k-aji-dev

Copy link
Copy Markdown
Author

Closing as a duplicate. #30 by @Fire162 already covers issue #29 and was opened first — that one should get the review, not this.

My mistake: I didn't check the open PR queue for an existing claim before sending this. Sorry for the noise.

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.

ruff check fails on main in playground/backend_manager

1 participant