Skip to content
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

Add Console Scripts for Cleaner, More Simple Script Execution #513

Closed
wants to merge 3 commits into from

Conversation

apockill
Copy link

What this does

This PR adds console script entry points in pyproject.toml using Poetry's [tool.poetry.scripts] section. This enables running scripts with simple commands like lr_train instead of python lerobot/scripts/train.py. All documentation has been updated accordingly, and the original method of running scripts remains functional.

Benefits

  • Users can run scripts with short, legible commands without needing to specify the python command or the script's file path.

  • Removing hardcoded file paths from documentation and docstrings decouples the documentation from the project's directory structure, facilitating future refactors.

  • Scripts can now be executed from any directory, not just the repository's root.

  • Low risk of breakage

How it was tested

  • I ran each new script command to ensure they execute correctly.
  • Verified that the original script execution method still works.

How to checkout & try? (for the reviewer)

# Ensure you have the changes to your poetry environment
poetry install

# Activate the virtual environment
poetry shell

# Run a script using the new command
lr_train --some-option

# Alternatively, use `poetry run`
poetry run lr_visualize_dataset --help

# Confirm the original method still works
python lerobot/scripts/train.py --some-option

@apockill
Copy link
Author

apockill commented Nov 18, 2024

This PR is ready for review! 😁

I took the liberty of choosing a naming pattern of lr_SCRIPT_NAME. I figure it'll make future refactors easier to be able to search lr_.* and see all usages in the codebase.

Also it's nice because you can type lr_ and then tab complete to see a complete list of scripts offered by lerobot.

@apockill
Copy link
Author

I'd love to know if this is a desired feature. If it's viewed as too invasive, I could revert all the changes except the pyproject.toml and have the scripts runnability be just another convenient way to run scripts instead of "the recommended path".

@Cadene
Copy link
Collaborator

Cadene commented Nov 25, 2024

Thanks for your PR. Imo, our API is not mature enough to include alias/shortcut for now. We prefer to be explicit and to have only one way of doing things for now. But I totally understand your reasoning.

If you think some elements are worth adding now, please open a new PR ;)
Sorry for the trouble!

@apockill
Copy link
Author

apockill commented Nov 26, 2024

That makes sense! Let's scrap this till a time when you feel the API is more stable.

My only worry is that the API is currently fairly hardcoded in the form of 10s of guides and documents. The benefit of this PR is we could keep the scripts as lr_blah_blah but change the underlying script they point to, creating an abstraction layer between what the documentation refers to, and the actual scripts that are run.

@apockill apockill closed this Nov 26, 2024
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.

2 participants