Skip to content

Conversation

tetsugo02
Copy link

@tetsugo02 tetsugo02 commented Aug 16, 2025

Added type hint and ensure the lerobot/configs modules pass the mypy check

Fixes #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.

#pyproject.toml
# uncomment the follow block
[tool.mypy]
python_version = "3.10"
warn_return_any = true
warn_unused_configs = true
ignore_missing_imports = false
# .pre-commit-config.yaml
# uncomment the fellow block
  - repo: https://github.com/pre-commit/mirrors-mypy
    rev: v1.16.0
    hooks:
      - id: mypy
        args: [--python-version=3.10, --follow-imports=silent]
        files: ^src/lerobot/configs/
        additional_dependencies:
          ["numpy", "torch", "huggingface_hub", "draccus"]

How it was tested

pre-commit

In this pr, I mainly fixed the mypy error, you can test the changes by using:

pre-commit run mypy --all-files 

mypy

mypy --check-untyped-defs  src/lerobot/configs/ 

Added type: ignore

  • src/lerobot/configs/train:
    • Used third-party library uses Any as return type
  • src/lerobot/configs/train
    • The parent class TrainPipelineConfig defined the dataset' type as DatasetConfig. And this property is used in many places. It was difficult to change.
  • src/lerobot/configs/policies
    • The parent class having a same name method makes here error, but the push_to_hub is also used in many places.

@tetsugo02 tetsugo02 changed the title Fix/typehint fix Fix typehint and address the mypy errors of src/lerobot/configs Aug 16, 2025
@tetsugo02 tetsugo02 marked this pull request as ready for review August 16, 2025 16:22
@Copilot Copilot AI review requested due to automatic review settings August 16, 2025 16:22
Copy link
Contributor

@Copilot Copilot AI left a 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.

@tetsugo02 tetsugo02 marked this pull request as draft August 22, 2025 15:10
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
@tetsugo02 tetsugo02 marked this pull request as ready for review August 24, 2025 15:21
@tetsugo02
Copy link
Author

Hello,
I am new to OSS, and this is my first attempt at contributing to this project.
I would appreciate any feedback. Thanks.

add args: --follow-imports=silent to pass error which have no relationship with src/lerobot/configs
Copy link
Member

@AdilZouitine AdilZouitine left a 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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ensure the configuration module passes MyPy type checks
2 participants