Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 6, 2025

What does this PR do?

Completes migration from mypy to pyright by removing all mypy configuration and enabling comprehensive type checking in standard mode.

Changes

Configuration (pyproject.toml)

  • Removed [tool.mypy] section entirely
  • Updated pyright comments to remove mypy-parity references
  • Enabled all previously disabled checks: reportAssignmentType, reportCallIssue, reportIndexIssue, reportGeneralTypeIssues

Type Fixes

  • fts.py:_reduce_transition: Fixed variable shadowing where decision parameter was reused for intermediate Tensor
  • fts_supporters.py:_lr_scheduler_sanity_chk: Changed LR scheduler instantiation from keyword to positional argument with type ignore for dynamic class validation
  • fts_supporters.py:_repartition_sharded_optim: Added type ignore for ZeroRedundancyOptimizer partition indexing (runtime-validated API)

Code Quality

  • Fixed spelling: "Recieved" → "Received" in error messages

Result

Pyright passes with 0 errors, 0 warnings in standard mode with comprehensive type checking enforcement.

Does your PR introduce any breaking changes? If yes, please list them.

None

Before submitting

  • Was this discussed/approved via a GitHub issue? (not for typos and docs)
  • Did you read the contributor guideline, Pull Request section?
  • Did you make sure your PR does only one thing, instead of bundling different changes together?
  • Did you make sure to update the documentation with your changes? (if necessary)
  • Did you write any new necessary tests? (not for typos and docs)
  • Did you verify new and existing tests pass locally with your changes?
  • Did you list all the breaking changes introduced by this pull request?
  • Did you update the CHANGELOG? (not for typos, docs, test updates, or internal minor changes/refactorings)

PR review

Anyone in the community is welcome to review the PR.
In short, see the following bullet-list:

  • Is this pull request ready for review? (if not, please submit in draft mode)
  • Check that all items from Before submitting are resolved
  • Make sure the title is self-explanatory and the description concisely explains the PR
  • Add labels and milestones (and optionally projects) to the PR so it can be classified

Did you have fun?

Make sure you had fun coding 🙃


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI changed the title [WIP] Complete Pyright migration and remove mypy configuration Complete Pyright Migration and Remove Mypy Configuration Dec 6, 2025
Copilot AI requested a review from speediedan December 6, 2025 21:38
@speediedan speediedan marked this pull request as ready for review December 6, 2025 21:48
Copilot AI review requested due to automatic review settings December 6, 2025 21:48
@speediedan speediedan merged commit daae2dc into mypy_pyright_conversion_sec_enhance Dec 6, 2025
6 checks passed
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR successfully completes the migration from mypy to pyright for type checking by removing all mypy configuration and enabling comprehensive type checking in pyright's standard mode. The changes include necessary code fixes to pass the stricter type checking requirements.

Key Changes:

  • Removed all mypy configuration from pyproject.toml and enabled comprehensive type checking in pyright (reportGeneralTypeIssues, reportAssignmentType, reportCallIssue, reportIndexIssue all set to "error")
  • Fixed variable shadowing in _reduce_transition where the decision parameter was being reused as a tensor variable
  • Corrected spelling errors: "Recieved" → "Received" in 4 error messages
  • Added targeted type ignore comments with specific error codes for runtime-validated patterns

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
pyproject.toml Removed complete [tool.mypy] section (28 lines) and updated pyright configuration to enable comprehensive type checking by setting reportGeneralTypeIssues, reportAssignmentType, reportCallIssue, and reportIndexIssue to "error"; updated comments to remove mypy-parity references
src/finetuning_scheduler/fts.py Fixed variable shadowing in _reduce_transition method by introducing separate variable names (decision_tensor, reduced_decision) instead of reusing the decision parameter
src/finetuning_scheduler/fts_supporters.py Corrected spelling of "Received" in 4 error messages; changed LR scheduler instantiation from keyword to positional argument (consistent with PyTorch API); added type ignore comments for dynamic class validation and ZeroRedundancyOptimizer partition indexing

decision = bool(strategy.reduce(decision, reduce_op=ReduceOp.SUM)) # type:ignore[arg-type]
return decision
decision_tensor = torch.tensor(int(decision), device=strategy.root_device)
reduced_decision = bool(strategy.reduce(decision_tensor, reduce_op=ReduceOp.SUM)) # type:ignore[arg-type]
Copy link

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style inconsistency: Use # type: ignore[arg-type] (with space) instead of # type:ignore[arg-type] (without space) to match the project's existing convention used in most other type ignore comments.

Suggested change
reduced_decision = bool(strategy.reduce(decision_tensor, reduce_op=ReduceOp.SUM)) # type:ignore[arg-type]
reduced_decision = bool(strategy.reduce(decision_tensor, reduce_op=ReduceOp.SUM)) # type: ignore[arg-type]

Copilot uses AI. Check for mistakes.
speediedan added a commit that referenced this pull request Dec 6, 2025
… CI Req Generation Security (#23)

* refactor: move torch-only lockfile pruning to Python helper

- Add `requirements/utils/prune_torch_deps.py`:
  - Implements iterative, transitive pruning of packages that are only required by `torch` (e.g., `networkx`, `sympy`, `mpmath`).
  - Provides a CLI: `python prune_torch_deps.py <lockfile_path>`.
  - Parses uv lockfile `# via` comments and prunes exclusive transitive deps using safe iterations.

- Simplify `requirements/utils/lock_ci_requirements.sh`:
  - Remove complex awk-based pruning logic and replace it with a simple call:
    `python "${SCRIPT_DIR}/prune_torch_deps.py" "${lockfile}"`.
  - Keep nightly lockfile generation behavior and the two-step manual install workflow.
  - Use `--index` (replaces deprecated `--extra-index-url`) for PyTorch nightly resolution.

- Security & maintenance:
  - Document security rationale: restrict the dependency confusion attack surface by pruning torch-only packages while using `--index-strategy unsafe-best-match` during lockfile generation only.
  - Keep installation workflow secure: manual two-step install or explicit nightly index for runtime.

- Tests & checks:
  - All unit tests pass (164 passed, 52 skipped).
  - Pre-commit hooks pass.
  - Module placed in `requirements/utils/` — not included in `src/` coverage.

Files changed:
- Added: `requirements/utils/prune_torch_deps.py`
- Updated: `requirements/utils/lock_ci_requirements.sh`
- Updated: `requirements/ci/requirements.txt` and `requirements/ci/requirements-oldest.txt` (torch-only deps pruned)

Notes:
- This refactor improves maintainability and makes pruning logic easier to test and extend.
- No change to the user-install workflow; nightly torch is still installed separately as before.

* initial copilot setup steps workflow, improved CI locked reqs logic, set pyright mode to standard and remove initial warnings

* Complete Pyright Migration and Remove Mypy Configuration (#24)

* Initial plan

* Remove mypy configuration and update pyright comments

Co-authored-by: speediedan <[email protected]>

* Enable reportAssignmentType check and fix variable shadowing in _reduce_transition

Co-authored-by: speediedan <[email protected]>

* Enable reportCallIssue check and fix LR scheduler instantiation

Co-authored-by: speediedan <[email protected]>

* Enable reportIndexIssue check and fix sharded optimizer indexing

Co-authored-by: speediedan <[email protected]>

* Enable reportGeneralTypeIssues and reorganize pyright config comments

Co-authored-by: speediedan <[email protected]>

* Fix spelling: Recieved -> Received in error messages

Co-authored-by: speediedan <[email protected]>

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: speediedan <[email protected]>

* fix docstring typo

---------

Co-authored-by: Copilot <[email protected]>
Co-authored-by: speediedan <[email protected]>
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.

2 participants