Type hints for Board + Move; mypy CI workflow (advisory) (#46)#54
Open
dbqpdb wants to merge 2 commits into
Open
Type hints for Board + Move; mypy CI workflow (advisory) (#46)#54dbqpdb wants to merge 2 commits into
dbqpdb wants to merge 2 commits into
Conversation
First pass of #46, scoped to the two most-depended-on classes per the issue's suggested order. Adds: - `from __future__ import annotations` so annotations are lazy and Move/Board forward references work without string-quoting them. - Module-level type aliases: SquareName (for the flexible inputs that square_name_to_array_idxs accepts), BoardIdxs (file_idx, rank_idx tuple), PieceChar (single piece letter). - Parameter and return annotations on every method of Board and Move. Annotations only — no behavior changes. All four TestRLM cases pass plus test_entered_move_processing. Follow-up passes will cover Piece + subclasses and Game/GameController. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Third CI workflow file, mirroring the lint workflow's pattern: - Triggers on every push and every pull_request. - continue-on-error: true so findings surface without blocking merge. - --ignore-missing-imports keeps the workflow runnable without installing numpy/pandas type stubs. Runs on rlm.py only for now; can widen once tests/ has type hints too. Current mypy reports ~60 errors against the partially-typed codebase — several map to bugs already filed separately (e.g. the setdefault issue in #33, the tuple-assert in #32, and __eq__'s object signature vs. body that assumes Move). The advisory mode means these accumulate as a backlog rather than blocking merge. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
First pass of #46, plus its CI complement. Two commits:
cfd079f— type hints on Board and Move. Annotates every method in the two most-depended-on classes per Cleanup: add complete type hints across the codebase #46's suggested order. Adds three module-level aliases (SquareName,BoardIdxs,PieceChar) for the recurring fuzzy types, and turns onfrom __future__ import annotationsso forward references and modern union syntax just work.518616d— mypy CI workflow. Third CI workflow file (typecheck.yml), mirrors the lint workflow pattern: runs on every push and PR, advisory (continue-on-error: true), uses--ignore-missing-importsso numpy/pandas stubs aren't needed.Annotations only — no behavior changes
All four
TestRLMcases pass;test_entered_move_processingruns to completion with the same result as before.Mypy report (current state)
Locally: 60 errors reported by
mypy rlm.py --ignore-missing-imports. The advisory workflow surfaces these in CI without blocking merge. Several map cleanly to existing bugs that are tracked separately:setdefault(..., None)polluting class-level dict — already filed as Bug: square_name_to_array_idxs uses setdefault, polluting class-level dict #33.assert(cond, "msg")always-true — already filed as Bug: assert(cond, 'msg') tuple form is always truthy in TestRLM #32.__eq__signature isobject(mypy-conventional) but the body accessesmove_to_match.single_char— flags up the missingisinstanceguard. Could be a follow-up.The bulk of the remaining errors fall into two buckets:
move_elem_dictinparse_move_without_gameis inferred asdict[str, str]from initialization, but getsboolandNonevalues assigned later. Fix is a single: dict[str, Any]annotation; intentionally not bundled into this PR to keep it pure-annotation-only.square_name_to_array_idxsreturnstuple[int | None, int | None], which is accurate (it returnsNonefor invalid inputs), but most call sites assume non-None. This pattern would benefit from a refactor — either raise on invalid input, or have callers handleNoneexplicitly.What's still out of scope
Pieceand its subclasses,Game,GameController,Player,Lexicon,Loudmouth. Per Cleanup: add complete type hints across the codebase #46, these come in follow-up passes.Closes #46 partially; will leave the issue open until the rest of the codebase is annotated.
🤖 Generated with Claude Code