-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Chore/mypy policies rebased #2295
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
akacmazz
wants to merge
8
commits into
huggingface:main
Choose a base branch
from
akacmazz:chore/mypy-policies-rebased
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
+151
−107
Conversation
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
This commit enables strict MyPy type checking for the policies module and adds comprehensive type annotations to ACT, Diffusion, and TDMPC policies. Changes: - pyproject.toml: Enable strict MyPy for policies module - ACT policy (11 type hints): Added return type annotations to __init__, reset, and helper methods; fixed Callable return type - Diffusion policy (13 type hints): Added return type annotations, fixed **kwargs typing, added parameter types to helper classes - TDMPC policy (8 type hints): Added return type annotations to __init__, reset, update, and internal methods Related to issue huggingface#1720
Added 8 return type annotations to SAC policy: - Main SACPolicy class: __init__, reset - Update methods: update_target_networks, update_temperature - Helper classes: MLP, CriticHead, CriticEnsemble, DiscreteCritic, Policy Related to issue huggingface#1720
Added 10 return type annotations total (5 each for Pi0 and Pi05): - Pi0Policy and PI05Policy classes: __init__, reset - PI0Pytorch and PI05Pytorch: __init__ - PaliGemmaWithExpertModel: __init__ with typed parameters - GemmaConfig: __init__ with all int parameters Related to issue huggingface#1720
Added 9 return type annotations total: - SmolVLA (3 type hints): SmolVLAPolicy __init__ and reset, VLAFlowMatching __init__ - VQBeT (6 type hints): VQBeTPolicy __init__ and reset, SpatialSoftmax, VQBeTModel, VQBeTHead, VQBeTRgbEncoder Related to issue huggingface#1720
Fixed 113 MyPy errors (131 → 18): - Added type annotations for queues and action queues across all policies - Fixed None type narrowing with assertions in ACT, Diffusion, SAC, VQBeT - Fixed return type annotations (get_optim_params) - Added 'from typing import Any' imports where needed - Enabled MyPy subclassing of Any types in pyproject.toml Remaining 18 errors are complex edge cases in SAC, Pi0/Pi05, SmolVLA. Related to issue huggingface#1720
Fixed all remaining 18 MyPy errors: - Added type annotations for inputs_embeds and adarms_cond in Pi0/Pi05 - Fixed variable redefinition errors in SAC by removing duplicate type annotations - Fixed SmolVLA forward return type (dict → tuple[Tensor, dict]) - Added type: ignore for intentional cases (torch.compile, getattr, resize_with_pad) - Added missing 'from typing import Any' imports RESULT: MyPy now passes with 0 errors on all 8 policy files! Starting point: 131 errors → Final: 0 errors (100% reduction) Related to issue huggingface#1720
Fixed all remaining 18 MyPy errors: - Added type annotations for inputs_embeds and adarms_cond in Pi0/Pi05 - Fixed variable redefinition errors in SAC by removing duplicate type annotations - Fixed SmolVLA forward return type (dict → tuple[Tensor, dict]) - Added type: ignore for intentional cases (torch.compile, getattr, resize_with_pad) - Added missing 'from typing import Any' imports RESULT: MyPy now passes with 0 errors on all 8 policy files! Starting point: 131 errors → Final: 0 errors (100% reduction) Related to issue huggingface#1720
Added TODO(huggingface#1720) comments with clear explanations for all 12 type: ignore instances as required by Issue huggingface#1720. Changes: - Diffusion (6 instances): MyPy cannot infer **kwargs types when unpacking dict - Pi0 (2 instances): config.dtype Literal mismatch, torch.compile method assignment - Pi05 (1 instance): config.dtype Literal mismatch - SAC (2 instances): Intentional forward() override, getattr runtime type resolution - SmolVLA (1 instance): MyPy cannot infer tuple unpacking with * operator All type: ignore comments now include: - TODO(huggingface#1720) reference linking to the issue - Clear explanation of why type: ignore is necessary MyPy verification: Success - no issues found in 49 source files
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What this does
This PR makes the
lerobot.policiesmodule 100% MyPy compliant by enabling strict type checking and adding comprehensive type annotations to all policy implementations.Addresses #1719 ; fixes #1720 . (🔧 Enhancement)
Changes:
MyPy Configuration (
pyproject.toml):disallow_subclassing_any = falsefor nn.Module compatibilityBase Classes (
src/lerobot/policies/pretrained.py):ClassVarannotations forconfig_classandnameattributesget_optim_params()return type todict | list[dict]for flexibilityPolicy Implementations (50+ type hints added across 8 files):
__init__,reset, helper methods**kwargstyping, added parameter types__init__,reset,updatemethodsType Checking Fixes:
type: ignorecomments with TODO(Ensure the policy module passes MyPy type checks #1720) explanations for genuine edge cases:**kwargsunpacking limitations (6 instances in Diffusion)How it was tested
1. MyPy Type Checking (Primary Test):