Skip to content

CHORE Add pyright config#51

Draft
felixdivo wants to merge 5 commits into
benchopt:mainfrom
felixdivo:chore/pyright-config
Draft

CHORE Add pyright config#51
felixdivo wants to merge 5 commits into
benchopt:mainfrom
felixdivo:chore/pyright-config

Conversation

@felixdivo

@felixdivo felixdivo commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Fixes some small mistakes found via type checking.

Also adds the Pyright config and a small workflow (which, however, is allowed to fail without holding anyone back). This config allows verifying types locally without much setup. Just run pyright once installed.

felixdivo and others added 2 commits June 13, 2026 23:18
Set up pyright (basic mode) and triage its initial 133 errors. ~125 were
structural false positives from benchopt's metaclass (`parameters` injected
as instance attributes; `name` overriding a base property) and untyped
third-party libs (tslearn, wfdb, torch, aeon, pandas).

Config (pyproject.toml) carries the suppression so the source stays clean (no
inline `# pyright: ignore` directives):
- mute reportAssignmentType project-wide (only ever fires on the benchopt
  `name`-overrides-base-property pattern);
- scope the remaining framework/stub-noise rules to the thin wrapper dirs
  `datasets/` and `solvers/` via executionEnvironments (extraPaths=["."] so
  first-party imports still resolve); benchmark_utils/ and objective.py stay
  strict, and high-signal rules (reportReturnType, reportRedeclaration, ...)
  stay on everywhere. benchmark_utils still catches real attribute typos.

Genuine fixes pyright found:
- linear_probe.py: `self.alpha = (alpha,)` wrapped the scalar in a 1-tuple and
  passed it to `RidgeClassifier(alpha=...)` (wants a float) — drop the tuple.
- naive.py: `_MajorityClassifier.predict` annotated `-> int` but returns a list.
- download.py: guard the `dict.get(path.name)` results (str | None) so an
  unknown subdir raises a clear ValueError instead of `"*" + None`.
- base_solver.py: widen `y_train` to `Sequence[np.ndarray] | None` (it may be
  None for unsupervised tasks) and drop the `supported_tasks` annotation that
  shadowed the abstract property.

Remaining stub residuals handled with plain Python, not pyright directives:
`_ZERO_DIV: Any` for sklearn's mistyped `zero_division`, an `Any` param on the
constant fallback adapter, and an `assert self.cutoff_indexes is not None`
documenting the forecasting invariant.

chronos.py keeps 3 reportGeneralTypeIssues ("Module cannot be used as a type":
chronos exports `ChronosPipeline` in a way pyright resolves to a module under
the package's lazy transformers imports) — left as known untyped-library noise.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Runs `pyright` on PRs/pushes with continue-on-error: true, so type
regressions in benchmark_utils/ and objective.py surface without gating the
build. Model/solver deps aren't installed in this lightweight job, so
missing-import noise is expected — hence non-blocking for now.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@felixdivo felixdivo self-assigned this Jun 13, 2026
@felixdivo felixdivo added the bug Something isn't working label Jun 13, 2026
@felixdivo felixdivo marked this pull request as draft June 13, 2026 23:36
felixdivo and others added 3 commits June 15, 2026 09:23
Install every non-torch dependency (pandas, tslearn, huggingface_hub, fsspec,
pooch, tqdm, wfdb) in the type_check job so benchmark_utils/, datasets/ and
objective.py are fully import-resolvable and meaningfully type-checked. The
torch-based model solvers (chronos/chronos2/mantis/moment/toto2) stay
uninstalled to keep the job fast; their missing imports are expected, so
reportMissingImports is muted in the solvers/ pyright exec-environment.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A completed Actions job can only be success or failure, so a non-blocking
check still rendered red. Make the pyright step always exit 0 and surface
findings as warning annotations (orange) instead — the job shows green, never
blocks a merge, and issues remain visible inline. Keep continue-on-error as a
safety net for install/infra failures.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@felixdivo

Copy link
Copy Markdown
Contributor Author

Warnings are shown like this:

Screenshot 2026-06-15 at 13 39 08

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant