Skip to content

Adds hydra safty guard against cli args starting with -- #4802

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.

Sign up for GitHub

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

Closed
ooctipus wants to merge 1 commit into from
Closed

Adds hydra safty guard against cli args starting with -- #4802

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions source/isaaclab_tasks/isaaclab_tasks/utils/hydra.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,8 @@ def parse_overrides(args: list[str], presets: dict) -> tuple:
global_presets, preset_sel, preset_scalar, global_scalar = [], [], [], []

for arg in args:
if arg.startswith("-"):
continue
Comment on lines +341 to +342

@greptile-apps greptile-apps Bot Mar 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Args starting with - are silently dropped with no logging or warning. If a user accidentally prefixes a Hydra override with a dash (e.g., -env.sim.dt=0.01 instead of env.sim.dt=0.01), the override will be silently ignored with no feedback as to why it had no effect.

Consider adding a debug-level log message to help users diagnose such situations:

Suggested change
if arg.startswith("-"):
continue
if arg.startswith("-"):
# Skip non-Hydra CLI args (e.g. --headless, --device=cpu)
continue

Adding a comment explaining this behavior would also help future maintainers understand the intent.

Comment on lines +341 to +342

@greptile-apps greptile-apps Bot Mar 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The PR description and title refer to filtering -- (double-dash) CLI args, but the guard uses startswith("-"), which also filters single-dash args (e.g., -v, -e). While filtering both single- and double-dash args is correct (neither is valid Hydra override syntax), the code comment should reflect this accurately to avoid confusing future maintainers.

Consider clarifying the intent with a more precise comment:

Suggested change
if arg.startswith("-"):
continue
if arg.startswith("-"):
# Skip standard UNIX-style CLI flags (both -flag and --flag).
# These are intended for the outer argparse layer, not for Hydra.
continue

if "=" not in arg:
global_scalar.append(arg)
continue
Expand Down
Loading