Address/cleanup pyright type issues throughout the codebase: Phase 1#153
Address/cleanup pyright type issues throughout the codebase: Phase 1#153speediedan merged 4 commits intomainfrom
Conversation
Co-authored-by: speediedan <7462936+speediedan@users.noreply.github.com>
|
No CI pin file changes detected by regen-ci-req-check. |
|
@copilot try again after your error |
I've resolved the network timeout errors that were preventing dependency installation. The pyright type checking expansion is now working correctly: ✅ Pyright passes: 0 errors, 2 warnings (acceptable optional dependency imports) The changes are ready and all validation that can run in this environment passes successfully. Commit: 81bf0b9 |
There was a problem hiding this comment.
Pull Request Overview
This PR implements Phase 1 of a systematic pyright type checking expansion, addressing issue #137 by increasing type checking coverage from 1 to 9 files with zero errors. The changes focus on fixing type issues in utility modules and expanding pyright configuration to enable gradual adoption of static type checking across the codebase.
Key changes:
- Fixed type errors in logging and exception handling utilities
- Expanded pyright configuration to include 8 additional files
- Updated documentation with GPU pipeline and dependency management guidance
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/interpretune/utils/logging.py |
Fixed potential None reference and added type ignore for deliberate function attribute assignment |
src/interpretune/utils/exceptions.py |
Added explicit type annotations and fixed potential None access from frame inspection |
pyproject.toml |
Expanded pyright include list from 1 to 9 files for incremental type checking adoption |
.github/copilot-instructions.md |
Added documentation for Azure GPU pipeline, dependency regeneration, and type-checking caveats |
| # add the attribute to the function but don't overwrite if it already exists | ||
| rank_zero_only.rank = getattr(rank_zero_only, "rank", _get_rank() or 0) | ||
| # type: ignore[attr-defined] - we're deliberately adding an attribute to a function | ||
| rank_zero_only.rank = getattr(rank_zero_only, "rank", _get_rank() or 0) # type: ignore[attr-defined] |
There was a problem hiding this comment.
[nitpick] The type ignore comment is duplicated. The comment at line 99 already explains the deliberate pattern, so the inline comment should just be # type: ignore[attr-defined] without the explanation.
|
Completed first phase of pyright type checking expansion addressing issue #137.
✅ Phase 1 Complete: Initial Pyright Expansion
Expanded pyright coverage from 1 → 9 files (900% increase)
All files under pyright checking now have 0 errors
Files now under pyright checking (9 total, all with 0 errors):
src/interpretune/adapters/lightning.py: 0 errors (already configured)src/interpretune/adapters/core.py: 0 errors (added)src/interpretune/adapter_registry.py: 0 errors (added)src/interpretune/utils/data_movement.py: 0 errors (added)src/interpretune/utils/tokenization.py: 0 errors (added)src/interpretune/utils/logging.py: 0 errors (fixed 2 errors)src/interpretune/utils/exceptions.py: 0 errors (fixed 10 errors)src/interpretune/utils/warnings.py: 0 errors (added)src/interpretune/utils/schema_validation.py: 0 errors, 2 warnings (added)Type fixes implemented:
utils/logging.py (2 errors → 0 errors):
orig_fnpotentially being None in context manager finally blockutils/exceptions.py (10 errors → 0 errors):
code_contextbeing potentially None from frame inspectionDict[str, Any]type annotations to resolve type inference issuesCurrent status:
Files assessed for future phases:
Phase 2 candidates (moderate complexity):
src/interpretune/utils/import_utils.py: 8 errors (type inference issues)src/interpretune/adapters/registration.py: 7 errors (dict typing issues)src/interpretune/session.py: 3 errors (parameter naming issues)src/interpretune/base/call.py: 2 errors (tensor type issues)Phase 3 candidates (complex):
src/interpretune/base/components/cli.py: 24 errors (complex CLI typing)src/interpretune/base/components/mixins.py: 35 errors (ML tensor operations)src/interpretune/base/components/core.py: 10 errors (protocol compatibility)src/interpretune/config/shared.py: 11 errors (generic type issues)src/interpretune/config/analysis.py: 12 errors (complex protocols)src/interpretune/registry.py: 15 errors (registry type system)Methodology validated:
# type: ignorefor deliberate patternsNext steps for subsequent PRs:
This establishes a solid foundation for continued incremental pyright expansion.
Fixes #137.
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.