-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Fix typehint and address the mypy errors of src/lerobot/configs #1746
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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 fixes type hint issues and addresses mypy errors in the src/lerobot/configs
module to ensure the codebase passes mypy static type checking.
- Adds proper type annotations to function return types and variables
- Fixes mypy errors related to assignment and type mismatches
- Improves error handling by raising exceptions instead of printing messages
- Adds null checks and proper handling for optional parameters
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
src/lerobot/configs/train.py | Adds type hints, improves validation logic with null checks, and fixes assignment issues |
src/lerobot/configs/policies.py | Adds type annotations and converts print statement to proper exception raising |
src/lerobot/configs/parser.py | Adds return type annotations and null safety checks for argument filtering |
src/lerobot/configs/eval.py | Adds null safety checks for policy attribute access |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
added typehint and addressed the error of mypy
I find that there are other dependencies of push_to_hub so I fix the property name back to original one.
As the copilot said, use raise before `hf_hub_download` would stop program even it is able to download
06ef1f4
to
86455f7
Compare
Hello, |
add args: --follow-imports=silent to pass error which have no relationship with src/lerobot/configs
86455f7
to
e2b857c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, I'm waiting the review of one more team member to approve and precommit hook pass!
Thanks for the contribution!
Added type hint and ensure the
lerobot/configs
modules pass the mypy checkFixes #1730
Changes
In this pr, I mainly fixed the mypy error, so I test my changes by using mypy and pre-commit
I changed pyproject.toml and .pre-commit-config as follows.
How it was tested
pre-commit
In this pr, I mainly fixed the mypy error, you can test the changes by using:
mypy
Added
type: ignore
Any
as return typeTrainPipelineConfig
defined the dataset' type asDatasetConfig
. And this property is used in many places. It was difficult to change.push_to_hub
is also used in many places.