added ruff test
This commit is contained in:
ykcheungab
@@ -6,19 +6,20 @@ All published NumPy arrays must be C-contiguous and read-only (frozen dataclasse
|
||||
alone do not freeze ndarray contents). Shapes are contractual, not type-checked
|
||||
by NDArray. See studio-3d-design.md for threading and validation requirements.
|
||||
"""
|
||||
from collections.abc import Callable, Mapping
|
||||
from concurrent.futures import Executor
|
||||
from dataclasses import dataclass
|
||||
from pathlib import Path
|
||||
from typing import Callable, Generic, Literal, Mapping, Protocol, TypeAlias, TypeVar
|
||||
from concurrent.futures import Executor
|
||||
from typing import Generic, Literal, Protocol, TypeAlias, TypeVar
|
||||
|
||||
import numpy as np
|
||||
from numpy.typing import NDArray
|
||||
from orisys.finger.runtime import FingerRuntime
|
||||
from orisys_app_shared.acquisition.models import SdkPacket
|
||||
from PySide6.QtCore import QEvent, SignalInstance
|
||||
from PySide6.QtGui import QMouseEvent, QOpenGLContext, QWheelEvent
|
||||
from PySide6.QtOpenGLWidgets import QOpenGLWidget
|
||||
from PySide6.QtWidgets import QWidget
|
||||
from orisys_app_shared.acquisition.models import SdkPacket
|
||||
from orisys.finger.runtime import FingerRuntime
|
||||
|
||||
F32: TypeAlias = NDArray[np.float32]
|
||||
F64: TypeAlias = NDArray[np.float64]
|
||||
@@ -305,7 +306,6 @@ class LatestFrameMailbox(Generic[T]):
|
||||
def __init__(self) -> None: ...
|
||||
def offer(self, frame: T) -> OfferResult:
|
||||
"""Thread-safe capacity-one replacement, constant-time under the lock."""
|
||||
...
|
||||
def take(self) -> T | None: ...
|
||||
def clear(self) -> None: ...
|
||||
@property
|
||||
@@ -323,7 +323,6 @@ class OrbitCamera(Protocol):
|
||||
|
||||
def configure_surface_format(profile: Profile) -> None:
|
||||
"""Before QApplication and before importing a platform-specific GL binding."""
|
||||
...
|
||||
|
||||
|
||||
class GLDispatch(Protocol):
|
||||
@@ -351,7 +350,6 @@ class Sensor3DWidget(QOpenGLWidget):
|
||||
parent: QWidget | None = None) -> None: ...
|
||||
def set_scene(self, assets: SceneAssets) -> None:
|
||||
"""GUI: retain CPU data, schedule upload when context is current."""
|
||||
...
|
||||
def present(self, frame: RenderFrame) -> None: ...
|
||||
def set_settings(self, settings: DisplaySettings) -> None: ...
|
||||
def initializeGL(self) -> None: ...
|
||||
|
||||
@@ -0,0 +1,74 @@
|
||||
# Code review: the automated part
|
||||
|
||||
Four gates existed before this document (the test suites, `apps/studio/tools/ui_check.py`, the
|
||||
real-GL smoke tests, the binding-purity test) and none of them is a linter. This adds the missing
|
||||
*static* layer, deliberately small.
|
||||
|
||||
## Why it exists, in one measured fact
|
||||
|
||||
Until now the three repos had **no Ruff configuration at all**. `ruff check .` therefore ran
|
||||
whatever the installed version enables by default — Ruff 0.16 enables a very broad set
|
||||
(`UP`/`RUF`/`PLR`/`S`/`TRY`/`PERF`/`FURB`/`YTT`/`ASYNC`…) — so the same tree produced **1038 /
|
||||
438 / 117** findings in `orisys-app` / `orisys-sdk` / `orisys-sensor-design`, and a different
|
||||
Ruff version produced different ones. A gate built on that is not a gate. `ruff.toml` now pins it.
|
||||
|
||||
## What is gated, and what is deliberately not
|
||||
|
||||
| rule | meaning | gated? | why |
|
||||
|---|---|---|---|
|
||||
| `BLE001` | broad `except Exception` | **no** | A broad catch is the *right* choice around device I/O, GL teardown and camera probing. This codebase already records that choice inline as `# noqa: BLE001 - <reason>`. Gating it would produce ~100 findings whose only correct fix is "add a reasoned suppression". |
|
||||
| `S110`, `S112` | handler is only `pass` / `continue` | **yes** | The complaint is not the breadth of the catch, it is that the failure becomes **invisible**. Silence is acceptable only with a reasoned suppression or a handler that reports/re-raises. |
|
||||
| `E722` | bare `except:` | **yes** | Strictly worse than `except Exception:` — it also swallows `KeyboardInterrupt`/`SystemExit`. |
|
||||
| `F` | unused import, redefinition, undefined name, unused variable | **yes** | Real defects: `F821`/`F822` are latent `NameError`s, `F811` can be a shadowed definition. |
|
||||
| `B023` | closure over a loop variable | **yes** | Late binding silently produces wrong geometry in the headless Blender scripts. |
|
||||
| `PLW1510` | `subprocess.run` without `check` | **yes** | A swallowed tool failure. |
|
||||
| `RUF100` | unused suppression | reported, **never auto-fixed** | The `- reason` in a `noqa` comment is documentation. A machine must not delete it. Triage by hand. |
|
||||
| `UP`, `I`, `PIE`, `RET501`, `PLR1711`, `SIM114`, `RUF022`, `RUF023` | mechanical modernisation | in the lint config, **not** in the gate | Applied once by `--fix`; gating them would only churn AI-authored diffs. |
|
||||
|
||||
`E402` (module import not at top of file) is **selected but not gated**. `import` after
|
||||
`os.environ[...]` is deliberate in this codebase — the environment must be set before Qt/cv2/the SDK
|
||||
load — so gating it would be pure noise. But it is the only rule that would catch an *accidental*
|
||||
import moved above that setup, so it stays visible. When you touch one of those files, record the
|
||||
intent as `# noqa: E402 - <reason>`; `E701`/`E702` are style, which `ruff format` owns. A gate that
|
||||
cries wolf gets ignored, which is worse than no gate.
|
||||
|
||||
## The three commands
|
||||
|
||||
```powershell
|
||||
# 1. The agent loop, after every edit — mechanical, no review needed.
|
||||
ruff check <changed files> --fix # never --unsafe-fixes
|
||||
ruff check <changed files> --output-format json # what remains, machine-readable
|
||||
|
||||
# 2. The gate before a change lands. Changed files only, machine-readable, non-zero on findings.
|
||||
python tools/lint_gate.py # vs HEAD
|
||||
python tools/lint_gate.py --base origin/main
|
||||
python tools/lint_gate.py --all # gate rules over the whole repo
|
||||
python tools/lint_gate.py --json
|
||||
|
||||
# 3. Formatting, changed files only — never a whole-tree rewrite mid-diff.
|
||||
ruff format <changed files>
|
||||
```
|
||||
|
||||
`$RUFF` may point at a specific ruff binary; otherwise `tools/lint_gate.py` uses `PATH` or the one
|
||||
beside the interpreter. **Pin the version**: the gate prints the version it ran, and findings differ
|
||||
between versions, so a runner must install the same one the developers use (currently 0.16.9).
|
||||
|
||||
## What this found when it was introduced
|
||||
|
||||
Filtering to rules that can indicate a defect produced ~18 candidates rather than style noise —
|
||||
which is the argument for the narrow set:
|
||||
|
||||
- six latent `NameError`s (`np`, `sys`, `Callable`, `FrameIdentity`, `tessellate_polygon`, and an
|
||||
`__all__` entry naming a function that does not exist) — none caught by any test;
|
||||
- a **duplicated** function definition that silently shadowed the first;
|
||||
- `lower == lower` / `upper == upper` in a range check, and the same pattern inside a **test
|
||||
assertion that could never fail**;
|
||||
- 13 closures over loop variables in the headless Blender/patch scripts;
|
||||
- two `subprocess.run` calls whose failure was silently ignored.
|
||||
|
||||
## Follow-up deliberately not done yet
|
||||
|
||||
`PLR0124` ("name compared with itself") is not in `select`. Eleven of its sites are the deliberate
|
||||
`value != value` NaN idiom and must become `math.isnan(...)` (or carry a reasoned suppression)
|
||||
before the rule can be gated — otherwise it is noise. Once those are converted, add `PLR0124` to
|
||||
`select` and to `GATE_SELECT` in `tools/lint_gate.py`.
|
||||
Reference in New Issue
Block a user