diff --git a/blender/scripts/patch_geometry.py b/blender/scripts/patch_geometry.py index 8f8b919..8e50d4c 100644 --- a/blender/scripts/patch_geometry.py +++ b/blender/scripts/patch_geometry.py @@ -41,7 +41,12 @@ def clip_halfspace(points, distance): def clip_rect(points, x0, x1, y0, y1): for axis, bound, sign in ((0, x0, 1), (0, x1, -1), (1, y0, 1), (1, y1, -1)): - points = clip_halfspace(points, lambda p: sign * (p[axis] - bound)) + # Bind the loop variables as defaults: the callback must clip against the + # plane of *this* iteration, not whatever the loop left behind. + points = clip_halfspace( + points, + lambda p, axis=axis, bound=bound, sign=sign: sign * (p[axis] - bound), + ) return points @@ -56,7 +61,12 @@ def project_triangle(clip_vertices): for i, v in enumerate(clip_vertices)] for axis in range(3): for sign in (-1, 1): - points = clip_halfspace(points, lambda p: p[3] + sign * p[axis]) + # Same late-binding hazard as clip_rect: bind axis/sign per iteration so + # the clip plane cannot be read from a later loop state. + points = clip_halfspace( + points, + lambda p, axis=axis, sign=sign: p[3] + sign * p[axis], + ) return [((p[0] / p[3] + 1) / 2, (p[1] / p[3] + 1) / 2, 1 / p[3], *(b / p[3] for b in p[4:])) for p in points if p[3] > 1e-15] diff --git a/blender/scripts/patch_projection.py b/blender/scripts/patch_projection.py index d30b512..dc47864 100644 --- a/blender/scripts/patch_projection.py +++ b/blender/scripts/patch_projection.py @@ -33,7 +33,11 @@ def build_patch_geometry(obj, cam, scene, depsgraph, uv_names, nx, ny, edge_samp output[name]['mesh'].append(uvs[name]) cache = {} - def visible(weights): + # ``cache`` and ``world`` are bound as defaults: ``cache`` is rebound + # for every loop triangle, and a late-bound closure would read the + # next triangle's cache (or the final triangle's, if a probe were + # deferred), yielding a wrong visibility answer per triangle. + def visible(weights, cache=cache, world=world): nonlocal ray_count key = tuple(round(w, 12) for w in weights) if key in cache: @@ -64,10 +68,10 @@ def build_patch_geometry(obj, cam, scene, depsgraph, uv_names, nx, ny, edge_samp output[name]['patches'].append((i, j, polygon)) print(f'Evaluated {len(mesh.loop_triangles)} loop triangles; {ray_count} visibility rays; ' f'{len(output[uv_names[0]]["patches"])} local fragments per UV target') - print(f'Visibility sampling: seven probes per fragment; uniformly probed ' - f'fragments are accepted or dropped whole, mixed boundary fragments ' - f'refine to 1/4 cell or depth 6 and unresolved leaves are rejected. ' - f'Sub-sample occluders may be missed; hit tolerance max(1e-7, distance*2e-5).') + print('Visibility sampling: seven probes per fragment; uniformly probed ' + 'fragments are accepted or dropped whole, mixed boundary fragments ' + 'refine to 1/4 cell or depth 6 and unresolved leaves are rejected. ' + 'Sub-sample occluders may be missed; hit tolerance max(1e-7, distance*2e-5).') return output finally: evaluated.to_mesh_clear() diff --git a/blender/scripts/probe_falloff.py b/blender/scripts/probe_falloff.py index 4d39762..d6258cc 100644 --- a/blender/scripts/probe_falloff.py +++ b/blender/scripts/probe_falloff.py @@ -31,7 +31,8 @@ bpy (the same pattern as ``blender/scripts/patch_geometry.py``). from __future__ import annotations -from typing import Any, Sequence +from collections.abc import Sequence +from typing import Any import numpy as np diff --git a/blender/scripts/random_patch_dd02c.py b/blender/scripts/random_patch_dd02c.py index 0681a2f..cd06387 100644 --- a/blender/scripts/random_patch_dd02c.py +++ b/blender/scripts/random_patch_dd02c.py @@ -38,6 +38,7 @@ sys.path.insert(0, str(Path(__file__).resolve().parent)) from patch_geometry import clip_rect from patch_projection import build_patch_geometry + # ---------------------------------------------------------------------- # Common Utilities # ---------------------------------------------------------------------- diff --git a/blender/scripts/test_patch_geometry.py b/blender/scripts/test_patch_geometry.py index 67e5e36..3f2aebf 100644 --- a/blender/scripts/test_patch_geometry.py +++ b/blender/scripts/test_patch_geometry.py @@ -1,7 +1,15 @@ """Run with python -m unittest discover -s blender/scripts -p test_patch_geometry.py.""" import unittest -from patch_geometry import (area, barycentric, cell_fragments, clip_rect, - interpolate_uv, project_triangle, visible_triangles) + +from patch_geometry import ( + area, + barycentric, + cell_fragments, + clip_rect, + interpolate_uv, + project_triangle, + visible_triangles, +) class GeometryTests(unittest.TestCase): diff --git a/blender/scripts/test_probe_falloff.py b/blender/scripts/test_probe_falloff.py index 3d6b16a..2eeaa1e 100644 --- a/blender/scripts/test_probe_falloff.py +++ b/blender/scripts/test_probe_falloff.py @@ -15,7 +15,6 @@ import base64 import numpy as np import pytest - from probe_falloff import ( FALLOFF_MODES, affected_bands, diff --git a/docs/code-review.md b/docs/code-review.md new file mode 100644 index 0000000..66c7191 --- /dev/null +++ b/docs/code-review.md @@ -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 - `. 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 - `; `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 --fix # never --unsafe-fixes +ruff check --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 +``` + +`$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`. diff --git a/ruff.toml b/ruff.toml new file mode 100644 index 0000000..0500a13 --- /dev/null +++ b/ruff.toml @@ -0,0 +1,55 @@ +# Ruff configuration — pinned deliberately. +# +# Before this file existed the repo had no Ruff config, so `ruff check .` ran whatever the +# installed Ruff version enables by default (Ruff 0.16 turns on a very broad set). Findings +# were therefore not reproducible between machines. Never rely on tool defaults. +# +# Policy, decided from the measured findings: +# +# * `select` is explicit: high-signal correctness rules first, then the mechanical +# modernisation rules only. +# * BLE001 is selected so existing reasoned suppressions stay *used* and RUF100 cannot +# delete their rationale. Broad catches around Blender/environment probing are legitimate. +# BLE001 is NOT part of the gate. +# * S110 / S112 / E722 ARE part of the gate. The Blender scripts are the riskiest place for +# a silent swallow: they run headless, so a swallowed error becomes a silently wrong or +# missing artefact rather than a visible crash. +# * B023 (loop-variable closure) matters here more than anywhere else in the workspace — +# the geometry/patch scripts define callbacks inside loops over axes and signs. +# * RUF100 is `unfixable`: a machine must not remove a suppression whose comment records +# why it exists. +# * The mechanical sweep never uses `--unsafe-fixes`. +# +# The gate selects the narrow set explicitly: +# +# tools/lint_gate.py # the gate: changed files, machine-readable + +target-version = "py310" +line-length = 88 + +[lint] +select = [ + "E9", "E722", # syntax/IO errors, and bare `except:` + "E402", # module import not at top of file — SELECTED for visibility, NOT gated. + # `import` after `os.environ[...]` is deliberate where the environment or + # the SDK path must be set first. E402 is the only rule that would catch an + # accidental import moved above that setup, so it stays reported without + # failing a build. An intentional site belongs in `# noqa: E402 - `. + "F", # pyflakes: unused import, redefinition, undefined name + "S110", "S112", # try-except-pass / try-except-continue — an invisible failure + "B023", # function definition does not bind loop variable + "PLW1510", # subprocess.run without an explicit check + "RUF100", # unused noqa — reported for triage, never auto-removed + "UP", # pyupgrade + "I", # import sorting + "RUF022", "RUF023", # __all__ / __slots__ ordering + "PIE", # flake8-pie + "RET501", "PLR1711", + "SIM114", + "BLE001", # selected for suppression hygiene only — excluded from the gate +] +unfixable = ["RUF100"] + +[format] +quote-style = "double" +indent-style = "space" diff --git a/sensors/finger/r01/source/geometry.blend b/sensors/finger/r01/source/geometry.blend index 5435ea5..d5bd9ce 100644 Binary files a/sensors/finger/r01/source/geometry.blend and b/sensors/finger/r01/source/geometry.blend differ diff --git a/src/orisys_sensor_design/cli.py b/src/orisys_sensor_design/cli.py index 3c832da..23122f8 100644 --- a/src/orisys_sensor_design/cli.py +++ b/src/orisys_sensor_design/cli.py @@ -16,8 +16,8 @@ from orisys_sensor_design.manifest import ( build_export_job, build_manifest, default_manifest_path, - export_revision_dir, export_output_dir, + export_revision_dir, load_profile, repo_root, validate_revision_metadata, @@ -662,6 +662,7 @@ def cmd_optics(revision_dir: Path, args: argparse.Namespace) -> int: ) return 2 + from orisys_sensor_design.optics import REPORT_FILENAME as OPTICS_REPORT_FILENAME from orisys_sensor_design.optics import ( OpticsConfigError, build_report, @@ -672,7 +673,6 @@ def cmd_optics(revision_dir: Path, args: argparse.Namespace) -> int: summarize, write_report, ) - from orisys_sensor_design.optics import REPORT_FILENAME as OPTICS_REPORT_FILENAME want_plot = not args.no_plot if want_plot: diff --git a/src/orisys_sensor_design/compare.py b/src/orisys_sensor_design/compare.py index c8dacdf..ee20f93 100644 --- a/src/orisys_sensor_design/compare.py +++ b/src/orisys_sensor_design/compare.py @@ -32,9 +32,10 @@ supplies one backed by the real stage helpers. from __future__ import annotations import json +from collections.abc import Callable, Mapping, Sequence from dataclasses import dataclass, field from pathlib import Path -from typing import Any, Callable, Mapping, Sequence +from typing import Any SCHEMA = "orisys-sensor-design/settings-comparison-v1" FILENAME = "settings_comparison.json" @@ -410,7 +411,10 @@ def _probe_for( changes["pattern"] = dataclasses_module.replace( pattern, nx=int(entry.nx), ny=int(entry.ny), seed=int(entry.seed) ) - except Exception: # a non-dataclass pattern must not break the sweep + except Exception: # noqa: S110 - a non-dataclass pattern must not break the sweep + # Deliberate: ``dataclasses.replace`` only applies to dataclass + # patterns. A duck-typed/plain pattern keeps its defaults and the + # sweep continues; the run is still recorded under its own settings. pass return dataclasses_module.replace(settings, **changes) @@ -651,7 +655,6 @@ def build_figures( __all__ = [ "COMPARISON_DIRNAME", - "CompareError", "DEFAULT_DIS_MODES", "DEFAULT_GRID", "DEFAULT_MAX_RUNS", @@ -659,6 +662,7 @@ __all__ = [ "DEFAULT_SEEDS", "FILENAME", "SCHEMA", + "CompareError", "SweepEntry", "SweepOutcome", "build_figures", diff --git a/src/orisys_sensor_design/deform.py b/src/orisys_sensor_design/deform.py index b4cbe2b..4b5e2b9 100644 --- a/src/orisys_sensor_design/deform.py +++ b/src/orisys_sensor_design/deform.py @@ -26,10 +26,10 @@ Contract: ``docs/probe-pipeline.md``. from __future__ import annotations import json -from collections.abc import Mapping +from collections.abc import Mapping, Sequence from dataclasses import dataclass, field from pathlib import Path -from typing import Any, Sequence +from typing import Any from orisys_sensor_design.manifest import RevisionPaths, export_object_names, load_yaml from orisys_sensor_design.texture import ( diff --git a/src/orisys_sensor_design/floorcal.py b/src/orisys_sensor_design/floorcal.py index 8999add..80ac99f 100644 --- a/src/orisys_sensor_design/floorcal.py +++ b/src/orisys_sensor_design/floorcal.py @@ -25,9 +25,10 @@ a 500x500 render. That ratio is the number the bench should be calibrated agains from __future__ import annotations +from collections.abc import Mapping, Sequence from dataclasses import dataclass from pathlib import Path -from typing import Any, Iterable, Mapping, Sequence +from typing import Any import numpy as np diff --git a/src/orisys_sensor_design/flow.py b/src/orisys_sensor_design/flow.py index dae171f..4943434 100644 --- a/src/orisys_sensor_design/flow.py +++ b/src/orisys_sensor_design/flow.py @@ -78,9 +78,10 @@ from __future__ import annotations import json import math +from collections.abc import Callable, Mapping, Sequence from datetime import datetime, timezone from pathlib import Path -from typing import Any, Callable, Mapping, Sequence +from typing import Any import numpy as np @@ -342,7 +343,9 @@ class FixedReferenceFlow: if callable(getter): try: return int(getter()) - except Exception: # pragma: no cover - defensive + except Exception: # noqa: S110 - defensive fallback to the documented default + # A binding whose getPatchSize() is missing or returns a non-numeric + # value reports the measured default instead of failing introspection. pass return DEFAULT_DIS_PATCH_PX @@ -1706,8 +1709,11 @@ def run_track_job( scale_output_dir = out / FLOW_DIRNAME / name scale_output_dir.mkdir(parents=True, exist_ok=True) - def _save(flow_array: np.ndarray, frame_name: str) -> None: - np.save(scale_output_dir / f"{frame_name}.npy", flow_array.astype(np.float32)) + # Bind the per-scale directory: it is reassigned for every ``name`` in + # the loop, so a late-bound closure would write every frame into the + # last scale's directory if these calls were ever deferred. + def _save(flow_array: np.ndarray, frame_name: str, out_dir: Path = scale_output_dir) -> None: + np.save(out_dir / f"{frame_name}.npy", flow_array.astype(np.float32)) _save(estimator.compute(reference), str(reference_entry.get("name", "reference"))) if null_path is not None: diff --git a/src/orisys_sensor_design/optimise.py b/src/orisys_sensor_design/optimise.py index dde4b6a..df8ea0b 100644 --- a/src/orisys_sensor_design/optimise.py +++ b/src/orisys_sensor_design/optimise.py @@ -19,10 +19,12 @@ this module imports neither Blender nor cv2 and is testable in isolation. from __future__ import annotations import json +import math +from collections.abc import Callable, Mapping, Sequence from dataclasses import dataclass from datetime import datetime, timezone from pathlib import Path -from typing import Any, Callable, Mapping, Sequence +from typing import Any from orisys_sensor_design.manifest import RevisionPaths, repo_root from orisys_sensor_design.sensitivity import ( @@ -88,19 +90,19 @@ ROI_CONFOUND_NOTE = ( ) __all__ = [ - "OPTIMIZER_REPORT_SCHEMA", - "OPTIMIZER_REPORT_FILENAME", - "OPTIMIZER_OBJECTIVE", "DEFAULT_GRID", - "DEFAULT_SEEDS", "DEFAULT_SEARCH_RESOLUTION", - "NOT_GLOBAL_OPTIMUM", + "DEFAULT_SEEDS", "GRID_RATIONALE", - "SCALE_DEPENDENT_NOTE", + "NOT_GLOBAL_OPTIMUM", + "OPTIMIZER_OBJECTIVE", + "OPTIMIZER_REPORT_FILENAME", + "OPTIMIZER_REPORT_SCHEMA", "ROI_CONFOUND_NOTE", - "OptimiseError", + "SCALE_DEPENDENT_NOTE", "Candidate", "CandidateRun", + "OptimiseError", "enumerate_candidates", "rank_entries", "run_optimise", @@ -231,7 +233,7 @@ def _as_float(value: Any) -> float | None: number = float(value) except (TypeError, ValueError): return None - if number != number: # NaN + if math.isnan(number): return None return number diff --git a/src/orisys_sensor_design/preview/window.py b/src/orisys_sensor_design/preview/window.py index ae2cd28..1bee8f1 100644 --- a/src/orisys_sensor_design/preview/window.py +++ b/src/orisys_sensor_design/preview/window.py @@ -6,6 +6,7 @@ The viewport widget is kept separate from the chrome so it can be embedded later from __future__ import annotations +import sys from pathlib import Path from typing import Any @@ -132,7 +133,10 @@ def run_preview( """ from PyQt5.QtWidgets import QApplication - from orisys_sensor_design.preview.gl_view import PreviewGLWidget, configure_surface_format + from orisys_sensor_design.preview.gl_view import ( + PreviewGLWidget, + configure_surface_format, + ) app = QApplication.instance() created = app is None diff --git a/src/orisys_sensor_design/printability.py b/src/orisys_sensor_design/printability.py index 3853351..bd3689b 100644 --- a/src/orisys_sensor_design/printability.py +++ b/src/orisys_sensor_design/printability.py @@ -626,9 +626,7 @@ def analyse_map( "passed": bool(max_ratio is not None and max_ratio <= settings.max_sliver_ratio), }, ] - if not uniform: - verdict = "inconclusive" - elif not material: + if not uniform or not material: verdict = "inconclusive" else: verdict = "pass" if all(check["passed"] for check in checks) else "fail" diff --git a/src/orisys_sensor_design/probe_plot.py b/src/orisys_sensor_design/probe_plot.py index c3541dc..f34feef 100644 --- a/src/orisys_sensor_design/probe_plot.py +++ b/src/orisys_sensor_design/probe_plot.py @@ -34,8 +34,9 @@ comparison figure that quietly omits a failed setting is worse than no figure. from __future__ import annotations +from collections.abc import Mapping, Sequence from pathlib import Path -from typing import Any, Mapping, Sequence +from typing import Any import numpy as np diff --git a/src/orisys_sensor_design/render.py b/src/orisys_sensor_design/render.py index c9ce4ec..636b927 100644 --- a/src/orisys_sensor_design/render.py +++ b/src/orisys_sensor_design/render.py @@ -462,10 +462,10 @@ __all__ = [ "RENDER_JOB_SCHEMA", "RENDER_MANIFEST_SCHEMA", "REQUIRED_VIEW_TRANSFORM", + "ProbeConfigError", "RenderArtifactError", "RenderConfigError", "RenderPaths", - "ProbeConfigError", "build_render_job", "default_render_paths", "frame_name", diff --git a/src/orisys_sensor_design/sensitivity.py b/src/orisys_sensor_design/sensitivity.py index 51a92aa..5f7e529 100644 --- a/src/orisys_sensor_design/sensitivity.py +++ b/src/orisys_sensor_design/sensitivity.py @@ -20,14 +20,20 @@ from __future__ import annotations import json import math +from collections.abc import Callable, Mapping, Sequence from dataclasses import dataclass from datetime import datetime, timezone from pathlib import Path -from typing import Any, Mapping, Sequence +from typing import Any import numpy as np -from orisys_sensor_design.manifest import RevisionPaths, load_yaml, repo_root, sha256_file +from orisys_sensor_design.manifest import ( + RevisionPaths, + load_yaml, + repo_root, + sha256_file, +) from orisys_sensor_design.texture import texture_revision_rel SENSITIVITY_REPORT_SCHEMA = "orisys-sensor-design/sensitivity-report-v2" @@ -68,30 +74,29 @@ VERDICT_SEVERITY = {"ambiguous": -1, "unresolvable": 0, "marginal": 1, "resolvab RESOLVABLE_SNR = 3.0 __all__ = [ - "SENSITIVITY_REPORT_SCHEMA", - "SENSITIVITY_REPORT_FILENAME", "DIS_PATCH_PX", - "min_local_scale_px_per_mm", "MIN_LOCAL_SCALE_PX_PER_MM", - "SensitivityError", - "SensitivityConfigError", - "SensitivityDataError", + "SENSITIVITY_REPORT_FILENAME", + "SENSITIVITY_REPORT_SCHEMA", + "AxisScore", + "FrameFlow", "MissingArtifactError", "Roi", - "FrameFlow", - "AxisScore", - "ScaleScore", "ScaleInput", + "ScaleScore", + "SensitivityConfigError", + "SensitivityDataError", + "SensitivityError", + "build_report", + "default_probe_output_dir", + "load_scale_inputs", + "min_local_scale_px_per_mm", + "resolution_pair", + "resolve_probe_output_dir", + "run_sensitivity", "score_axis", "score_scale", - "build_report", - "collect_report", - "run_sensitivity", "write_sensitivity_report", - "load_scale_inputs", - "default_probe_output_dir", - "resolve_probe_output_dir", - "resolution_pair", ] diff --git a/tests/test_deform.py b/tests/test_deform.py index 5a1ba58..72cc61c 100644 --- a/tests/test_deform.py +++ b/tests/test_deform.py @@ -14,11 +14,11 @@ from orisys_sensor_design.deform import ( DEFAULT_PROBE, DEFORM_JOB_FILENAME, DEFORM_JOB_SCHEMA, - FRAMES_NPZ_FILENAME, DEFORM_REPORT_FILENAME, + FRAMES_NPZ_FILENAME, + PatternInfo, ProbeConfigError, ProbeFrame, - PatternInfo, build_deform_job, deform_job_path, frame_plan, diff --git a/tests/test_optics.py b/tests/test_optics.py index 59cc825..4251c53 100644 --- a/tests/test_optics.py +++ b/tests/test_optics.py @@ -9,12 +9,12 @@ import pytest np = pytest.importorskip("numpy") +from orisys_sensor_design.manifest import RevisionPaths, repo_root from orisys_sensor_design.optics import ( - AIR_INDEX, Interface, + OpticalStack, OpticsConfigError, OpticsLayer, - OpticalStack, intersect_mesh, load_optics_block, quantify, @@ -24,7 +24,6 @@ from orisys_sensor_design.optics import ( trace_ray, validate_optics_block, ) -from orisys_sensor_design.manifest import RevisionPaths, repo_root REPO = repo_root() diff --git a/tests/test_optics_plot.py b/tests/test_optics_plot.py index 2487b8a..98e36e4 100644 --- a/tests/test_optics_plot.py +++ b/tests/test_optics_plot.py @@ -2,7 +2,6 @@ from __future__ import annotations -import math import sys from pathlib import Path @@ -11,14 +10,13 @@ import pytest np = pytest.importorskip("numpy") from orisys_sensor_design.optics import ( - AIR_INDEX, Interface, - OpticsLayer, OpticalStack, + OpticsLayer, camera_rays, + quantify, sample_stack, summarize, - quantify, ) from orisys_sensor_design.optics_plot import ( DEFAULT_SANITY_MM, diff --git a/tests/test_printability.py b/tests/test_printability.py index 1ff8495..bdb41f3 100644 --- a/tests/test_printability.py +++ b/tests/test_printability.py @@ -341,7 +341,7 @@ def test_reference_cross_printable_flag(tmp_path: Path) -> None: _polygon("patch-1", "0,0 50,0 50,50 0,50") + "\n" + ' \n' - + f' \n' + " " ) diff --git a/tests/test_render.py b/tests/test_render.py index 32fae4e..56ed633 100644 --- a/tests/test_render.py +++ b/tests/test_render.py @@ -76,11 +76,14 @@ requires_blender = pytest.mark.skipif( def _run_blender(script: Path, job: Path, timeout: int = 900) -> subprocess.CompletedProcess: assert BLENDER is not None + # check=False is deliberate: several tests assert on non-zero Blender exit + # codes (2/3), so a non-zero status is data, not an error to raise on. return subprocess.run( [str(BLENDER), "-b", str(SOURCE_BLEND), "-P", str(script), "--", "--job", str(job)], capture_output=True, text=True, timeout=timeout, + check=False, ) @@ -248,7 +251,7 @@ def test_job_carries_the_hard_render_settings(paths, settings, tmp_path): assert job["objects"] == {"mesh": "sensing_surface", "camera": "cam_sensor"} assert job["null_frame"] == {"name": REFERENCE_NULL_NAME, "seed": 1} assert job["artifacts"]["report"] == str( - (Path(tmp_path).resolve() / render.MANIFEST_FILENAME) + Path(tmp_path).resolve() / render.MANIFEST_FILENAME ) assert job["artifacts"]["frames_npz"] == str( deform.probe_artifact_paths(Path(tmp_path))["frames"] diff --git a/tests/test_uv_map.py b/tests/test_uv_map.py index cdbca9b..8326913 100644 --- a/tests/test_uv_map.py +++ b/tests/test_uv_map.py @@ -195,7 +195,6 @@ def _stub_sdk(monkeypatch: pytest.MonkeyPatch, calls: dict, mask_mode: str = "va arrays["valid_mask"] = np.ones((4, 4), bool) arrays["world_xyz"] = np.zeros((4, 4, 3), np.float32) np.savez(map_path, **arrays) - return None monkeypatch.setattr(geometry, "scale_camera_geometry", fake_scale_camera_geometry) monkeypatch.setattr(precompute, "build_or_load_domain", fake_build_or_load_domain) @@ -306,7 +305,6 @@ def test_a_map_without_a_valid_mask_is_fatal( calls["build_kwargs"] = kwargs height, width = int(kwargs["out_height"]), int(kwargs["out_width"]) np.savez(map_path, map_x=np.zeros((height, width), np.float32)) - return None monkeypatch.setattr(precompute, "build_or_load_domain", build_without_mask) with pytest.raises(uv.UvMapError, match="valid_mask"): diff --git a/tools/lint_gate.py b/tools/lint_gate.py new file mode 100644 index 0000000..e1109b4 --- /dev/null +++ b/tools/lint_gate.py @@ -0,0 +1,193 @@ +#!/usr/bin/env python3 +"""Merge gate for lint on a change — the narrow rule set a change must pass. + +This is deliberately *not* the repo's full `ruff.toml` selection. The policy, decided from the +measured findings rather than from taste: + + * ``BLE001`` (blind except) is **allowed**. A broad ``except Exception`` is the right choice + around device I/O, GL teardown and camera probing, and this codebase already records that + choice inline as ``# noqa: BLE001 - ``. Gating it would produce ~100 findings whose + only correct fix is "add a reasoned suppression". + * ``S110``/``S112`` (a handler that is only ``pass``/``continue``) and ``E722`` (bare + ``except:``) **are** gated: 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. + * ``RUF100`` (unused suppression) is reported for triage but is configured ``unfixable``: a + machine must never delete a suppression whose comment records why it exists. + +Run over the files a change touches, never the whole tree, so an AI-authored diff is not +drowned in pre-existing style churn. + +Usage:: + + tools/lint_gate.py # files changed vs HEAD (staged + unstaged) + tools/lint_gate.py --base origin/main # files changed vs a ref + tools/lint_gate.py --files a.py b.py # explicit files + tools/lint_gate.py --all # gate rules over the whole repo + tools/lint_gate.py --json # machine-readable result on stdout + +Exit codes: 0 clean, 1 findings, 2 a usage or environment problem (e.g. ruff missing). +""" + +from __future__ import annotations + +import argparse +import json +import os +import shutil +import subprocess +import sys +from pathlib import Path + +#: The gate. Explicit, so a rule added to `ruff.toml` cannot silently start failing builds. +#: `E4`/`E7` are deliberately absent: `E402` fires on the intentional "set the environment before +#: importing Qt" pattern in scripts, and `E701`/`E702` are style, which `ruff format` handles. +GATE_SELECT = "E9,E722,F,S110,S112,B023,PLW1510" + +#: Rules we report but never let a tool auto-fix (RUF100 is `unfixable` in ruff.toml too). +REPORT_ONLY = "RUF100" + +REPO = Path(__file__).resolve().parents[1] + + +def find_ruff() -> str: + """Locate ruff: $RUFF, then PATH, then the interpreter environment.""" + for candidate in (os.environ.get("RUFF"), shutil.which("ruff")): + if candidate and Path(candidate).is_file(): + return candidate + beside = Path(sys.executable).parent / ("ruff.exe" if os.name == "nt" else "ruff") + if beside.is_file(): + return str(beside) + print( + "lint_gate: ruff not found. Set $RUFF, install it beside the interpreter, or put it on " + "PATH. The pin matters: findings differ between ruff versions, so the gate names its " + "version below when it runs.", + file=sys.stderr, + ) + raise SystemExit(2) + + +def git(*args: str) -> list[str]: + result = subprocess.run( + ["git", "-C", str(REPO), *args], + capture_output=True, + text=True, + check=False, + ) + if result.returncode != 0: + print(f"lint_gate: git {' '.join(args)} failed: {result.stderr.strip()}", file=sys.stderr) + raise SystemExit(2) + return [line for line in result.stdout.splitlines() if line.strip()] + + +def changed_python_files(base: str | None) -> list[str]: + """The .py files this change touches: working tree, index, and new files.""" + paths: set[str] = set() + if base: + paths.update(git("diff", "--name-only", "--diff-filter=ACMR", f"{base}...HEAD")) + paths.update(git("diff", "--name-only", "--diff-filter=ACMR")) + paths.update(git("diff", "--name-only", "--cached", "--diff-filter=ACMR")) + paths.update(git("ls-files", "--others", "--exclude-standard")) + return sorted(p for p in paths if p.endswith(".py") and (REPO / p).is_file()) + + +def run_ruff(ruff: str, args: list[str]) -> tuple[list[dict], str]: + version = subprocess.run( + [ruff, "--version"], capture_output=True, text=True, check=False + ).stdout.strip() + result = subprocess.run( + [ruff, "check", "--no-cache", "--output-format", "json", *args], + capture_output=True, + text=True, + check=False, + ) + if result.returncode not in (0, 1): + print(f"lint_gate: ruff failed ({result.returncode}): {result.stderr.strip()}", file=sys.stderr) + raise SystemExit(2) + try: + findings = json.loads(result.stdout or "[]") + except json.JSONDecodeError: + print(f"lint_gate: could not parse ruff output: {result.stdout[:400]}", file=sys.stderr) + raise SystemExit(2) + return findings, version + + +def describe(finding: dict) -> str: + code = finding.get("code") or "?" + location = finding.get("location") or {} + path = str(finding.get("filename", "")).replace(str(REPO) + os.sep, "") + return ( + f"{path}:{location.get('row', '?')}:{location.get('column', '?')}: {code}: " + f"{finding.get('message', '')}" + ) + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + parser.add_argument("--base", default=None, help="compare against this ref (e.g. origin/main)") + parser.add_argument("--files", nargs="*", default=None, help="explicit files to check") + parser.add_argument("--all", action="store_true", help="run the gate rules over the whole repo") + parser.add_argument("--json", action="store_true", help="machine-readable result on stdout") + args = parser.parse_args(argv) + + ruff = find_ruff() + + if args.all: + targets = [str(REPO)] + else: + targets = args.files or changed_python_files(args.base) + if not targets: + print("lint_gate: no changed Python files; nothing to check.") + if args.json: + print(json.dumps({"findings": [], "checked": [], "status": "clean"})) + return 0 + + findings, version = run_ruff(ruff, ["--select", GATE_SELECT, *targets]) + report_only, _ = run_ruff(ruff, ["--select", REPORT_ONLY, *targets]) + + if args.json: + print( + json.dumps( + { + "ruff": version, + "select": GATE_SELECT, + "checked": targets, + "findings": [ + { + "file": str(f.get("filename", "")).replace(str(REPO) + os.sep, ""), + "row": (f.get("location") or {}).get("row"), + "column": (f.get("location") or {}).get("column"), + "code": f.get("code"), + "message": f.get("message"), + "fixable": bool((f.get("fix") or {}).get("applicability") == "safe"), + } + for f in findings + ], + "unused_suppressions": [describe(f) for f in report_only], + "status": "findings" if findings else "clean", + }, + indent=2, + ) + ) + return 1 if findings else 0 + + print(f"lint_gate: {version}") + print(f" select {GATE_SELECT} (BLE001 is allowed by policy; see ruff.toml)") + print(f" checked {len(targets)} target(s)") + if findings: + print(f"\n{len(findings)} finding(s):") + for finding in sorted(findings, key=describe): + hint = " [safe fix: ruff check --fix]" if (finding.get("fix") or {}).get("applicability") == "safe" else "" + print(f" {describe(finding)}{hint}") + else: + print(" no findings") + if report_only: + print(f"\n{len(report_only)} unused suppression(s) to triage by hand (never auto-removed):") + for finding in report_only: + print(f" {describe(finding)}") + print(f"\nstatus: {'FINDINGS' if findings else 'clean'}") + return 1 if findings else 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tools/verify_xynova_texture.py b/tools/verify_xynova_texture.py index 30f410e..3c84709 100644 --- a/tools/verify_xynova_texture.py +++ b/tools/verify_xynova_texture.py @@ -36,6 +36,7 @@ import bpy import numpy as np from bpy_extras.object_utils import world_to_camera_view from mathutils import Vector +from mathutils.geometry import tessellate_polygon def sha256(path):