Skip to content

Conversation

@dhellmann
Copy link
Member

Summary

This PR enables comprehensive strict mypy type checking across the entire fromager codebase through an incremental approach. Each strict checking option was enabled in separate commits with corresponding fixes.

Changes Made

  • Expanded mypy coverage: Added tests/ and e2e/ directories to mypy checking
  • Enabled strict type checking options:
    • check_untyped_defs = true
    • disallow_incomplete_defs = true
    • disallow_untyped_defs = true
    • warn_return_any = true
  • Removed excluded source files: Previously excluded sources.py and commands/build.py now pass strict checking
  • Fixed type annotation issues: Added missing type annotations, resolved type mismatches, and added explicit type annotations to eliminate "returning Any" warnings

Testing

All mypy checks now pass with zero errors: Success: no issues found in 54 source files

🤖 Generated with Claude Code

Removes sdist.py and settings.py from mypy exclude list as these files
no longer exist in the codebase.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
@dhellmann dhellmann requested a review from a team as a code owner December 14, 2025 20:25
@mergify mergify bot added the ci label Dec 14, 2025
dhellmann and others added 9 commits December 14, 2025 15:53
Extends mypy configuration to include tests and e2e directories in
mypy_path and updates hatch mypy environment to check all three
directories for comprehensive type checking coverage.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
Enables check_untyped_defs to check function bodies even without type
annotations. Added exclusions for problematic e2e directories (build
directories and pyo3_test with missing imports). All existing code
passes this check without requiring fixes.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
Enables disallow_incomplete_defs to require complete type annotations
on function signatures. Fixed all issues in source code including:
- Added missing return type annotations
- Added missing parameter type annotations
- Fixed incompatible type issues in packagesettings.py
- Corrected return type for _add_node in dependency_graph.py

Source code now passes strict incomplete definition checking.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
Enables disallow_untyped_defs to require type annotations on all
function definitions. Fixed all issues in source code including:
- Added missing return type annotations (-> None for void functions)
- Added missing parameter type annotations
- Fixed BaseException vs Exception type for _format_exception
- Updated pydantic field validator with proper typing

Source code now passes strict untyped definition checking.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
Add explicit type annotations to resolve 'returning Any' warnings:
- finders.py: Add explicit pathlib.Path annotations for variables
- wheels.py: Add explicit pathlib.Path annotation for new_wheel_file
- bootstrapper.py: Add explicit pathlib.Path annotation for sdist_filename
- bootstrapper.py: Add return type annotation for _create_unpack_dir

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
Remove sources.py and commands/build.py from mypy exclude list.
Fix all type annotation issues in previously excluded files:

sources.py:
- Add null check for download URL
- Fix return types for _download_source_check and nested functions
- Fix function parameter and return type annotations
- Handle Version|None sorting in patch map iteration
- Convert Path arguments to str for external commands
- Add explicit type annotations to resolve Any returns

commands/build.py:
- Add type annotations for dict_factory and _create_table functions
- Add proper typing for **kwargs parameters

All source files now pass strict mypy type checking.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
… parameter

The serialize() method should default to a frozenset as it was before.
- Change exclude parameter type to set[str] | frozenset[str]
- Default to frozenset({'name', 'has_config'})
- Convert frozenset to set when calling model_dump() to satisfy Pydantic's IncEx type

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
Remove the instructions to the agent to use type annotations. We can
rely on the linter to catch issues.
- Add scripts/pre-commit hook that runs lint:check and mypy:check
- Add scripts/setup-pre-commit-hook.sh for easy installation
- Update CONTRIBUTING.md with setup and usage instructions
- Ensures all commits pass quality checks before being created

Co-Authored-By: Claude <[email protected]>
exit 1
fi

echo "✅ All pre-commit checks passed!" No newline at end of file
Copy link
Member

Choose a reason for hiding this comment

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

nit pick, we should add a new line at the end of file. Without it it messes up some editors

Copy link
Member Author

Choose a reason for hiding this comment

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

I will come back and add a separate linter to enforce that in another PR.

echo "before every commit to ensure code quality."
echo ""
echo "To bypass the hook for a specific commit (not recommended):"
echo " git commit --no-verify -m \"message\"" No newline at end of file
Copy link
Member

Choose a reason for hiding this comment

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

nit pick, we should add a new line at the end of file.

@LalatenduMohanty
Copy link
Member

Overall, this is awesome.

@LalatenduMohanty LalatenduMohanty merged commit baa8bfb into python-wheel-build:main Dec 16, 2025
124 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants