fix: ensure motors module passes MyPy type checks#2732
fix: ensure motors module passes MyPy type checks#2732yurekami wants to merge 4 commits intohuggingface:mainfrom
Conversation
This commit fixes 62 mypy type errors in the motors module by: - Updating Protocol classes (PortHandler, PacketHandler, GroupSyncRead, GroupSyncWrite) to use class-level attribute declarations instead of __init__ body declarations - Adding missing `broadcastPing` method to PacketHandler Protocol - Fixing return type annotations (e.g., `_get_motor_model` returns str, not int) - Fixing parameter types to use `Sequence` for covariant list parameters - Fixing `Mapping` for covariant dict value types in `_normalize` - Updating method signatures to be consistent across parent and child classes (disable_torque, enable_torque, _get_half_turn_homings) - Adding explicit `int()` casts for MotorCalibration arguments - Adding explicit `return None` for functions returning Optional types - Adding type annotations for variables like `data_list: dict[int, int]` - Using `# type: ignore[method-assign]` for intentional monkeypatch - Fixing variable references (using `self.groups` instead of `groups`) Fixes huggingface#1723 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
imstevenpmwork
left a comment
imstevenpmwork
left a comment
There was a problem hiding this comment.
Hello, thanks for contributing! Good job with this PR 😄
I left some minor comments, but they are mostly questions for my understanding.
Also, the linter CI is failing, could you please run pre-commit run -a?
Thanks again!
| return ids_values | ||
|
|
||
| def _get_half_turn_homings(self, positions: dict[NameOrID, Value]) -> dict[NameOrID, Value]: | ||
| def _get_half_turn_homings(self, positions: dict[str, Value]) -> dict[str, Value]: |
There was a problem hiding this comment.
Is the change for _get_half_turn_homings motivated from the fact that the function is only ever called with a dict[str,Value] as an input?
| return mins, maxes | ||
|
|
||
| def _normalize(self, ids_values: dict[int, int]) -> dict[int, float]: | ||
| def _normalize(self, ids_values: Mapping[int, Value]) -> dict[int, float]: |
There was a problem hiding this comment.
What's the motivation behind this change?
| model = self.motors[motor].model | ||
| addr, length = get_address(self.model_ctrl_table, model, data_name) | ||
|
|
||
| int_value: int |
There was a problem hiding this comment.
Could we do int_value = int(value) right away and delete the else down below ?
| model = next(iter(models)) | ||
| addr, length = get_address(self.model_ctrl_table, model, data_name) | ||
|
|
||
| int_ids_values: dict[int, int] |
There was a problem hiding this comment.
Maybe in here too? int_ids_values = {id_: int(val) for id_, val in raw_ids_values.items()} and remove the else block below
|
Hello @yurekami , have you had any time to check on this ? |
|
Superseded by: #2939 |
2 participants
Summary
lerobot.motorsmodulepyproject.tomlChanges Made
Protocol Classes (
motors_bus.py)PortHandler,PacketHandler,GroupSyncRead, andGroupSyncWriteProtocol classes to use class-level attribute declarations instead of__init__body declarationsbroadcastPingmethod toPacketHandlerProtocolType Annotations
_get_motor_model(returnsstr, notint)_get_ids_values_dict(returnsdict[int, Value], notlist[str])_readreturn type totuple[int, int, int](wastuple[int, int])_normalizeto acceptMapping[int, Value]for covarianceSequence[NameOrID]instead oflist[NameOrID]for covariant list parameters in public APIsMethod Signatures
enable_torquein base class to acceptint | str | list[str] | Noneto matchdisable_torque_get_half_turn_homingssignature to usedict[str, Value]consistentlyOther Fixes
return Nonefor functions returningOptionaltypesdata_list: dict[int, int]in_broadcast_pingint()casts forMotorCalibrationarguments# type: ignore[method-assign]for intentional monkeypatch in feetech.pyRangeFinderGUI.__init__(usingself.groupsinstead ofgroupsafter initialization)Test Plan
mypy src/lerobot/motors --show-error-codespasses with no errorsFixes #1723
🤖 Generated with Claude Code