Skip to content

chore: make the ruff lint gate pass - #3

Merged
emcramer merged 2 commits into
mainfrom
chore/lint-cleanup
Jul 21, 2026
Merged

chore: make the ruff lint gate pass#3
emcramer merged 2 commits into
mainfrom
chore/lint-cleanup

Conversation

@emcramer

Copy link
Copy Markdown
Owner

The lint job has never passed on main. ruff check . reports 6,130 errors there, and since the test job declares needs: lint, no PR has ever been able to run the test suite — including #2.

This PR gets ruff check . to zero without changing behavior. It should land before #2, which then rebases green.

This is not caused by #2

Branch Ruff errors
main 6,130
fix/physicell-reader-defects (#2) 5,050
this branch 0

#2 reduced the count by ~1,080. The CI failure it hit is inherited from main.

Why the obvious fix doesn't work

The suggestion going around is ruff check --fix tests/. Two problems:

  1. Wrong scope. CI runs ruff check ., not tests/. Most errors are in spatialtissuepy/.
  2. Autofix alone can't finish. Safe autofix takes 5,050 → 1,738. CI stays red. And a blanket --fix would strip 12 F401 re-exports out of __init__.py files, silently amputating the public API.

Configuration

  • select/ignore moved under [tool.ruff.lint] (ruff deprecated the top-level form — it was emitting a warning on every run).
  • Excluded tutorial notebooks and .ipynb_checkpoints/. Notebooks rely on cross-cell state ruff reads as undefined names; nbsphinx already exercises them at docs build.
  • Ignored UP006/UP007/UP045 with rationale in-file. The package still declares Python 3.8 support, and rewriting ~950 annotations to PEP 585/604 form is pure churn under from __future__ import annotations — and would have conflicted heavily with fix: PhysiCell reader orientation and label handling #2. Worth revisiting when the 3.8 floor drops (3.8 has been EOL since Oct 2024, and the CI matrix still tests it).
  • per-file-ignores for F401 in __init__.py.

Config changes alone: 6,130 → 4,963.

Real bugs the linter was correctly reporting

These are the reason this PR isn't purely mechanical:

  • Union used but never imported in lda/analysis.py, lda/metrics.py, network/clustering.py, network/communicability.py, statistics/colocalization.py, statistics/hotspots.py — 29 annotation sites across 7 modules. Latent only because from __future__ import annotations defers evaluation; it raises NameError under typing.get_type_hints(). Signature introspection does exactly that, which makes this a live risk for the MCP server.
  • plt referenced in module-level annotations in viz/config.py while only imported lazily inside functions. Added a TYPE_CHECKING import so matplotlib stays an optional runtime dependency.
  • SpatialLDA, MapperResult, SpatialTissueData used as forward references with no TYPE_CHECKING import.

Judgment calls

  • == Trueis True in test_physicell_io.py, not ruff's suggested bare truth check. These assertions deliberately verify a bool is returned; assert is_alive(0) would pass on any truthy value and weaken the test.
  • Optional-dependency probes (import networkx inside try) carry # noqa: F401 (optional dependency probe). The import being unused is the entire point.
  • Moran's I carries # noqa: E741 (Moran's I) at 4 sites. I = (n / W) * (numerator / denominator) is the statistic's canonical form; renaming it would obscure the formula.

One thing worth a look

topology/nerve.py:263 declared edge_set: Set[Tuple[int, int]] = set() # Track added edges and never used it. I removed it as dead code, but the comment suggests intended edge-deduplication that was never wired up. Not changing that here — flagging it in case nerve construction is emitting duplicate edges.

Verification

  • ruff check .All checks passed
  • 456 passed, 2 skipped — identical to main's baseline (the 2 skips are pre-existing optional-NetworkX cases)
  • Public API surface is byte-identical. I dumped every public symbol of every module via pkgutil.walk_packages before and after; diff is empty. This was the main risk of touching F401, so it's checked rather than assumed.

Sequencing

Merge this, then I'll rebase #2. There'll be a small conflict in synthetic/physicell/reader.py (both branches touch the import block) — trivial to resolve.

emcramer added 2 commits July 21, 2026 16:15
The lint job has never passed on main: `ruff check .` reported 6,130
errors, and because the test job declares `needs: lint`, no PR has been
able to run the test suite.

Fix the linter configuration and the code it flags:

- Move `select`/`ignore` under `[tool.ruff.lint]`, which ruff has
  deprecated the top-level form in favour of.
- Exclude tutorial notebooks and .ipynb_checkpoints. Notebooks rely on
  cross-cell state that ruff reads as undefined names; nbsphinx already
  exercises them at docs build time.
- Ignore UP006/UP007/UP045. The package still supports Python 3.8 and
  rewriting ~950 annotations to PEP 585/604 form is churn with no runtime
  effect under `from __future__ import annotations`.
- Per-file-ignore F401 in __init__.py, where imports are re-exports.

Genuine defects the linter was correctly reporting:

- `Union` was used in annotations but never imported in seven modules
  (lda, network, statistics). Deferred annotations kept this latent, but
  it raises NameError under typing.get_type_hints(), which signature
  introspection relies on.
- `plt` was referenced in module-level annotations in viz/config.py while
  only imported lazily inside functions; added a TYPE_CHECKING import that
  keeps matplotlib optional at runtime.
- `SpatialLDA`, `MapperResult` and `SpatialTissueData` were used as
  forward references without TYPE_CHECKING imports.

The rest is mechanical: trailing whitespace, import ordering, unused
locals, and `== True` to `is True` in assertions. Optional-dependency
probes and Moran's I variables carry documented noqa comments.

No behaviour change: 456 passed / 2 skipped, unchanged from main, and the
public API surface of every module is byte-identical.
With the lint gate passing, the test matrix ran for the first time and
every job failed at install:

  ERROR: Could not find a version that satisfies the requirement
  fastmcp<3,>=2.0; extra == "all"

The package declared `requires-python = ">=3.8"`, but the `mcp` extra
pulls in fastmcp, which requires 3.10 or newer. The two have never been
consistent; nothing surfaced it because no CI job had reached the install
step.

Python 3.8 reached end of life in October 2024 and 3.9 in October 2025.
The current releases of the core scientific stack have already moved past
both: numpy and scipy now require 3.12, and pandas, scikit-learn and
matplotlib require 3.11.

Raise requires-python to >=3.10, update the classifiers and the black,
ruff and mypy target versions to match, and change the CI matrix to
3.10-3.13.

Also ignore UP035 alongside the other annotation-style rules. Raising
ruff's target surfaced it, and it cannot be applied independently:
dropping the typing imports while the annotations still say `Dict[...]`
would not import.
@emcramer

Copy link
Copy Markdown
Owner Author

Pushed 23738a5: raise the Python floor to 3.10.

With the lint gate passing, the test matrix reached the install step for the first time and every job failed:

ERROR: Could not find a version that satisfies the requirement fastmcp<3,>=2.0; extra == "all"

requires-python declared >=3.8, but the mcp extra pulls in fastmcp, which requires 3.10+. Those two have never been consistent — nothing surfaced it because no CI job had ever gotten far enough to try.

Python 3.8 hit EOL in Oct 2024 and 3.9 in Oct 2025. The core scientific stack has already moved past both: current numpy and scipy require 3.12; pandas, scikit-learn and matplotlib require 3.11. The >=3.8 claim was untestable in principle, not just in CI.

Raised requires-python to >=3.10, aligned the classifiers and the black/ruff/mypy targets, and set the CI matrix to 3.10–3.13. Verified fastmcp 2.x has 70 releases installable on 3.10.

Raising ruff's target surfaced 120 UP035 (deprecated from typing import Dict). Added it to the same deferred group as UP006/UP007/UP045 — it can't be applied on its own, since dropping the typing imports while annotations still say Dict[...] wouldn't import. That modernisation is worth its own pass, not a side effect of this one.

@emcramer
emcramer merged commit 376b330 into main Jul 21, 2026
5 checks passed
@emcramer
emcramer deleted the chore/lint-cleanup branch July 21, 2026 23:34
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