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

Fix broken create_lerobot_dataset_card #590

Conversation

helper2424
Copy link
Contributor

What this does

During the recording of dataset for HIL-SERL integration I have found the following issue:

Traceback (most recent call last):.93M/4.54M [00:01<00:00, 4.89MB/s]
  File "/Users/helper2424/Documents/lerobot/lerobot/scripts/control_robot.py", line 577, in <module>      | 3/6 [00:03<00:04,  1.38s/it]
    record(robot, **kwargs)███████████| 6/6 [00:03<00:00,  1.88it/s]
  File "/Users/helper2424/Documents/lerobot/lerobot/common/robot_devices/utils.py", line 28, in wrapper
    raise e
  File "/Users/helper2424/Documents/lerobot/lerobot/common/robot_devices/utils.py", line 24, in wrapper
    return func(robot, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/helper2424/Documents/lerobot/lerobot/scripts/control_robot.py", line 334, in record
    dataset.push_to_hub(tags=tags)
  File "/Users/helper2424/Documents/lerobot/lerobot/common/datasets/lerobot_dataset.py", line 499, in push_to_hub
    card = create_lerobot_dataset_card(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/helper2424/Documents/lerobot/lerobot/common/datasets/utils.py", line 500, in create_lerobot_dataset_card
    return DatasetCard.from_template(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/helper2424/Library/Caches/pypoetry/virtualenvs/lerobot-_GqtL9sV-py3.12/lib/python3.12/site-packages/huggingface_hub/repocard.py", line 480, in from_template
    return super().from_template(card_data, template_path, template_str, **template_kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/helper2424/Library/Caches/pypoetry/virtualenvs/lerobot-_GqtL9sV-py3.12/lib/python3.12/site-packages/huggingface_hub/repocard.py", line 329, in from_template
    template_str = Path(template_path).read_text()
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/Cellar/python@3.12/3.12.7_1/Frameworks/Python.framework/Versions/3.12/lib/python3.12/pathlib.py", line 1027, in read_text
    with self.open(mode='r', encoding=encoding, errors=errors) as f:
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/Cellar/python@3.12/3.12.7_1/Frameworks/Python.framework/Versions/3.12/lib/python3.12/pathlib.py", line 1013, in open
    return io.open(self, mode, buffering, encoding, errors, newline)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
FileNotFoundError: [Errno 2] No such file or directory: '<contextlib._GeneratorContextManager object at 0x3181632f0>'

This is issue is relevant in the last main.

Examples:

Title Label
Fixes #[issue] (🐛 Bug)

How it was tested

How to test that issue is fixed:

  • Run data collection script, mine is the following:
python lerobot/scripts/control_robot.py record \
    --robot-path lerobot/configs/robot/koch_helper2424.yaml \
    --num-episodes 2 \
    --warmup-time-s 5 \
    --episode-time-s 10 \
    --fps 30 \
    --assign-rewards 1 \
    --policy-overrides "device=mps" \
    --repo-id ${HF_USER}/hil-serl-pusht-reward-classifier-test \
    --single-task "Push small T object to the correct position"
  • Run pytest tests/lerobot/common/datasets/test_utils.py

Copy link
Contributor

@jackvial jackvial left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested on my setup and it worked great. Here's the tests dataset I created https://huggingface.co/datasets/jackvial/create_lerobot_dataset_card_fix_test_0

The fix and unit tests make sense 👍

@michel-aractingi
Copy link
Collaborator

cc @aliberts for visibility

@aliberts
Copy link
Collaborator

Thank you @helper2424!

Could you expand on why using

card_template = (importlib.resources.files("lerobot.common.datasets") / "card_template.md").read_text()

DatasetCard.from_template(
    card_data=card_data,
    template_str=card_template,
)

rather than

card_template_path = importlib.resources.path("lerobot.common.datasets", "card_template.md")

DatasetCard.from_template(
    card_data=card_data,
    template_path=str(card_template_path),
)

fixed your issue? Both should be +/- equivalent.
It looked from your stack trace that the file wasn't there. Am I reading this wrong?

@helper2424
Copy link
Contributor Author

helper2424 commented Dec 22, 2024

@aliberts Hello, the file exists, the issue with the following - code str(card_template_path) returns '<contextlib._GeneratorContextManager object at 0x3181632f0>'. It return the introspection of the object instead of the path to the file. Also, I see that the contextlib generates a lot of warnings about the API usage, so I decided the way that doesn't generate warnings and that looks ok with their long term development way.

@helper2424
Copy link
Contributor Author

@aliberts in my PR I read the content of the file and pass it to the from_template.

@aliberts
Copy link
Collaborator

@helper2424 I can't seem to be able to reproduce you error. Currently, running this on main give me no error:

from lerobot.common.datasets.utils import create_lerobot_dataset_card

card = create_lerobot_dataset_card()
print(card)

And when running it with the debugger and inspecting str(card_template_path), I do get the correct path as a str.

After digging a bit, I think this is probably due to differences between our environments and how lerobot was installed, which could make importlib.resources behave differently. I also noticed that importlib.resources.path should be used as a context manager, which it's not here. Let's make this change then!

@aliberts
Copy link
Collaborator

Style test is failing, you can fix this by running our pre-commits.
Install pre-commit:

# using pip
pip install -e '.[dev]'

# or using poetry
poetry install --extras 'dev'

Run pre-commits

pre-commit run -a

Then commit & push

Copy link
Collaborator

@aliberts aliberts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@aliberts aliberts merged commit 59e2757 into huggingface:main Dec 23, 2024
5 checks passed
villekuosmanen added a commit to villekuosmanen/lerobot that referenced this pull request Dec 30, 2024
* feat: enable to use multiple rgb encoders per camera in diffusion policy (huggingface#484)

Co-authored-by: Alexander Soare <alexander.soare159@gmail.com>

* Fix config file (huggingface#495)

* fix: broken images and a few minor typos in README (huggingface#499)

Signed-off-by: ivelin <ivelin117@gmail.com>

* Add support for Windows (huggingface#494)

* bug causes error uploading to huggingface, unicode issue on windows. (huggingface#450)

* Add distinction between two unallowed cases in name check "eval_" (huggingface#489)

* Rename deprecated argument (temporal_ensemble_momentum) (huggingface#490)

* Dataset v2.0 (huggingface#461)

Co-authored-by: Remi <remi.cadene@huggingface.co>

* Refactor OpenX (huggingface#505)

* Fix missing local_files_only in record/replay (huggingface#540)

Co-authored-by: Simon Alibert <alibert.sim@gmail.com>

* Control simulated robot with real leader (huggingface#514)

Co-authored-by: Remi <remi.cadene@huggingface.co>

* Update 7_get_started_with_real_robot.md (huggingface#559)

* LerobotDataset pushable to HF from any folder (huggingface#563)

* Fix example 6 (huggingface#572)

* fixing typo from 'teloperation' to 'teleoperation' (huggingface#566)

* [vizualizer] for LeRobodDataset V2 (huggingface#576)

* Fix broken `create_lerobot_dataset_card`  (huggingface#590)

* feat(act): support training end of episode token to ACT model

* changes

* feat(arx): add arx arm (#2)

* feat(arx): support arx arm

* changes

* changes

* changes

* changes

* pass pipes explicitly

* changes

* us ndarray over a pipe

* changes

* changes

* replay basically works

* patch arx sdk

* changes

* support cameras in arx5

* rename to arx5

* kind of works

* changes

* changes

* changes

* various changes

* changes

* revert a few changes

* changes

* changes

* changes

* changes

* changes

* changes

* changes

* changes

* changes

* remove TODO

* allow multiple tasks

---------

Signed-off-by: ivelin <ivelin117@gmail.com>
Co-authored-by: Hirokazu Ishida <38597814+HiroIshida@users.noreply.github.com>
Co-authored-by: Alexander Soare <alexander.soare159@gmail.com>
Co-authored-by: Arsen Ohanyan <arsenohanyan@gmail.com>
Co-authored-by: Ivelin Ivanov <ivelin117@gmail.com>
Co-authored-by: Daniel Ritchie <daniel@brainwavecollective.ai>
Co-authored-by: resolver101757 <kelster101757@hotmail.com>
Co-authored-by: Jannik Grothusen <56967823+J4nn1K@users.noreply.github.com>
Co-authored-by: KasparSLT <133706781+KasparSLT@users.noreply.github.com>
Co-authored-by: Simon Alibert <75076266+aliberts@users.noreply.github.com>
Co-authored-by: Remi <remi.cadene@huggingface.co>
Co-authored-by: Michel Aractingi <michel.aractingi@huggingface.co>
Co-authored-by: Simon Alibert <alibert.sim@gmail.com>
Co-authored-by: berjaoui <berjaoui@gmail.com>
Co-authored-by: Claudio Coppola <Claudiocoppola90@gmail.com>
Co-authored-by: s1lent4gnt <kmeftah.khalil@gmail.com>
Co-authored-by: Mishig <dmishig@gmail.com>
Co-authored-by: Eugene Mironov <helper2424@gmail.com>
villekuosmanen added a commit to villekuosmanen/lerobot that referenced this pull request Jan 10, 2025
* feat: enable to use multiple rgb encoders per camera in diffusion policy (huggingface#484)

Co-authored-by: Alexander Soare <alexander.soare159@gmail.com>

* Fix config file (huggingface#495)

* fix: broken images and a few minor typos in README (huggingface#499)

Signed-off-by: ivelin <ivelin117@gmail.com>

* Add support for Windows (huggingface#494)

* bug causes error uploading to huggingface, unicode issue on windows. (huggingface#450)

* Add distinction between two unallowed cases in name check "eval_" (huggingface#489)

* Rename deprecated argument (temporal_ensemble_momentum) (huggingface#490)

* Dataset v2.0 (huggingface#461)

Co-authored-by: Remi <remi.cadene@huggingface.co>

* Refactor OpenX (huggingface#505)

* Fix missing local_files_only in record/replay (huggingface#540)

Co-authored-by: Simon Alibert <alibert.sim@gmail.com>

* Control simulated robot with real leader (huggingface#514)

Co-authored-by: Remi <remi.cadene@huggingface.co>

* Update 7_get_started_with_real_robot.md (huggingface#559)

* LerobotDataset pushable to HF from any folder (huggingface#563)

* Fix example 6 (huggingface#572)

* fixing typo from 'teloperation' to 'teleoperation' (huggingface#566)

* [vizualizer] for LeRobodDataset V2 (huggingface#576)

* Fix broken `create_lerobot_dataset_card`  (huggingface#590)

* Update README.md (huggingface#612)

* Fix Quality workflow (huggingface#622)

* fix(docs): typos in benchmark readme.md (huggingface#614)

Co-authored-by: Simon Alibert <75076266+aliberts@users.noreply.github.com>

* fix(visualise): use correct language description for each episode id (huggingface#604)

Co-authored-by: Simon Alibert <75076266+aliberts@users.noreply.github.com>

* typo fix: batch_convert_dataset_v1_to_v2.py (huggingface#615)

Co-authored-by: Simon Alibert <75076266+aliberts@users.noreply.github.com>

* [viz] Fixes & updates to html visualizer (huggingface#617)

* fixes to SO-100 readme (huggingface#600)

Co-authored-by: Philip Fung <no@one>
Co-authored-by: Simon Alibert <75076266+aliberts@users.noreply.github.com>

---------

Signed-off-by: ivelin <ivelin117@gmail.com>
Co-authored-by: Hirokazu Ishida <38597814+HiroIshida@users.noreply.github.com>
Co-authored-by: Alexander Soare <alexander.soare159@gmail.com>
Co-authored-by: Arsen Ohanyan <arsenohanyan@gmail.com>
Co-authored-by: Ivelin Ivanov <ivelin117@gmail.com>
Co-authored-by: Daniel Ritchie <daniel@brainwavecollective.ai>
Co-authored-by: resolver101757 <kelster101757@hotmail.com>
Co-authored-by: Jannik Grothusen <56967823+J4nn1K@users.noreply.github.com>
Co-authored-by: KasparSLT <133706781+KasparSLT@users.noreply.github.com>
Co-authored-by: Simon Alibert <75076266+aliberts@users.noreply.github.com>
Co-authored-by: Remi <remi.cadene@huggingface.co>
Co-authored-by: Michel Aractingi <michel.aractingi@huggingface.co>
Co-authored-by: Simon Alibert <alibert.sim@gmail.com>
Co-authored-by: berjaoui <berjaoui@gmail.com>
Co-authored-by: Claudio Coppola <Claudiocoppola90@gmail.com>
Co-authored-by: s1lent4gnt <kmeftah.khalil@gmail.com>
Co-authored-by: Mishig <dmishig@gmail.com>
Co-authored-by: Eugene Mironov <helper2424@gmail.com>
Co-authored-by: CharlesCNorton <135471798+CharlesCNorton@users.noreply.github.com>
Co-authored-by: Philip Fung <1054593+philfung@users.noreply.github.com>
Co-authored-by: Philip Fung <no@one>
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.

4 participants