Skip to content

fix(enjoy): restore --capture-video argument removed in #424#550

Open
mturan33 wants to merge 1 commit into
vwxyzjn:masterfrom
mturan33:fix/enjoy-capture-video-flag
Open

fix(enjoy): restore --capture-video argument removed in #424#550
mturan33 wants to merge 1 commit into
vwxyzjn:masterfrom
mturan33:fix/enjoy-capture-video-flag

Conversation

@mturan33
Copy link
Copy Markdown
Contributor

Description

cleanrl_utils/enjoy.py references args.capture_video at line 42 but the corresponding argparse definition was removed during the tyro refactor in #424 — despite the PR's name, enjoy.py was never actually migrated to tyro and remained on raw argparse. As a result, any python -m cleanrl_utils.enjoy ... invocation crashes with AttributeError: 'Namespace' object has no attribute 'capture_video' immediately after the HuggingFace Hub model download succeeds.

This PR restores the --capture-video flag using the exact same pattern as the training scripts (cleanrl/ppo.py, cleanrl/ddpg_continuous_action.py, cleanrl/sac_continuous_action.py): action="store_true", default False, byte-identical help text.

It also corrects --capture_video--capture-video in docs/get-started/CleanRL_Huggingface_Integration_Demo.ipynb (two occurrences). argparse does not normalize underscores to hyphens, so the documented command in the notebook would have failed even after the Python fix.

Verification

Reproduced the original crash on master: AttributeError: 'Namespace' object has no attribute 'capture_video'

After the fix:

  • python -m cleanrl_utils.enjoy --env-id CartPole-v1 --exp-name dqn --eval-episodes 1args.capture_video = False
  • python -m cleanrl_utils.enjoy --env-id CartPole-v1 --exp-name dqn --eval-episodes 1 --capture-videoargs.capture_video = True
  • python -m cleanrl_utils.enjoy --help → now lists --capture-video with the correct help text

Note: I used --exp-name dqn for repro because ppo is not a registered key in cleanrl_utils/evals/__init__.py. The argparse-level fix is exp-name-independent.

Out of scope (worth flagging separately)

  • enjoy.py is the last cleanrl_utils/ module still on raw argparse. A full tyro migration would be cleaner long-term but is intentionally not bundled here — mixing a refactor with a regression fix complicates review.
  • tests/test_enjoy.py exists but is not wired into any CI workflow and its commands are malformed (python enjoy.py --env ... instead of python -m cleanrl_utils.enjoy --env-id ...). This is why the regression went uncaught for ~15 months. Happy to open a separate issue for this.

Closes #497

Types of changes

  • Bug fix

Checklist:

  • I've read the CONTRIBUTION guide (required).
  • I have ensured pre-commit run --all-files passes (required).
  • No behavioral changes for users who don't pass --capture-video (default False matches pre-Refactor to use tyro #424 behavior).

cleanrl_utils/enjoy.py references args.capture_video at line 42 but the
corresponding argparse definition was deleted during the tyro refactor
(vwxyzjn#424). This causes any python -m cleanrl_utils.enjoy ... invocation
to crash with AttributeError after the HF Hub model download succeeds.

This restores the --capture-video flag with the same pattern used by
training scripts (action="store_true", default False).

Also corrects --capture_video → --capture-video in
docs/get-started/CleanRL_Huggingface_Integration_Demo.ipynb. argparse
does not normalize underscores to hyphens, so the documented command
in the notebook would have failed even after the Python fix.

Closes vwxyzjn#497
@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 20, 2026

@mturan33 is attempting to deploy a commit to the Costa Huang's projects Team on Vercel.

A member of the Team first needs to authorize it.

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.

--capture_video argument still referenced in enjoy.py

1 participant