Skip to content

fix: Use nargs for custom_bash_cmds#2261

Merged
ko3n1g merged 4 commits intomainfrom
ko3n1g/fix/custom-bash-cmds
Feb 6, 2026
Merged

fix: Use nargs for custom_bash_cmds#2261
ko3n1g merged 4 commits intomainfrom
ko3n1g/fix/custom-bash-cmds

Conversation

@ko3n1g
Copy link
Copy Markdown
Contributor

@ko3n1g ko3n1g commented Feb 6, 2026

What does this PR do ?

Add a one line overview of what this PR aims to accomplish.

Changelog

  • Add specific line by line info of high level changes in this PR.

GitHub Actions CI

See the CI sectionin the Contributing doc for how to trigger the CI. A Nvidia developer will need to approve and trigger the CI for external contributors.

Before your PR is "Ready for review"

Pre checks:

  • Make sure you read and followed Contributor guidelines
  • Did you write any new necessary tests?
  • Did you add or update any necessary documentation?
  • Does the PR affect components that are optional to install? (Ex: Numba, Pynini, Apex etc)
    • Reviewer: Does the PR have correct import guards for all optional libraries?

If you haven't finished some of the above items you can still open "Draft" PR.

Additional Information

  • Related to # (issue)

Summary by CodeRabbit

  • Refactor
    • Updated command argument processing to accept multiple separate inputs instead of a single delimited format.

Signed-off-by: oliver könig <okoenig@nvidia.com>
@ko3n1g ko3n1g requested review from a team, erhoo82 and malay-nagda as code owners February 6, 2026 14:23
@ko3n1g ko3n1g changed the title fix: Use nargs fix: Use nargs for custom_bash_cmds Feb 6, 2026
Signed-off-by: oliver könig <okoenig@nvidia.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Feb 6, 2026

📝 Walkthrough

Walkthrough

The -cb/--custom_bash_cmds command-line argument parsing is refactored from using a custom list_of_strings type that accepted a single comma-separated value to using argparse's native nargs="*" parameter, enabling users to pass multiple bash commands as separate arguments. The help text is updated to reflect this new behavior. Default remains an empty list.

Changes

Cohort / File(s) Summary
Argument Parser Update
scripts/performance/argument_parser.py
Changed custom_bash_cmds argument from comma-separated string parsing (type=list_of_strings) to argparse nargs="*" for direct multi-argument acceptance. Updated help text to clarify new usage pattern.

Possibly related PRs

Suggested labels

r0.3.0

Suggested reviewers

  • erhoo82
  • malay-nagda

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Test Results For Major Changes ✅ Passed PR contains only minor configuration changes that do not affect numerics, convergence, performance, or core functionality.
Title check ✅ Passed The title 'fix: Use nargs for custom_bash_cmds' directly and specifically describes the main change: replacing argparse type-based parsing with an nargs approach for the custom_bash_cmds option.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch ko3n1g/fix/custom-bash-cmds

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@ko3n1g ko3n1g added the r0.3.0 Cherry-pick label for r0.3.0 release branch label Feb 6, 2026
help="Comma separated string of bash commands",
nargs="*",
help="List of bash commands to execute before the main command",
default=[],
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.

Can we keep None as default or change the condition in scripts/performance/utils/executors.py that currently checks for None?

Signed-off-by: oliver könig <okoenig@nvidia.com>
Signed-off-by: oliver könig <okoenig@nvidia.com>
@ko3n1g ko3n1g merged commit 7dd2007 into main Feb 6, 2026
14 checks passed
@ko3n1g ko3n1g deleted the ko3n1g/fix/custom-bash-cmds branch February 6, 2026 15:28
ko3n1g added a commit that referenced this pull request Feb 6, 2026
Signed-off-by: oliver könig <okoenig@nvidia.com>
Signed-off-by: NeMo Bot <nemo-bot@nvidia.com>
sowmen pushed a commit to sowmen/Megatron-Bridge that referenced this pull request Feb 11, 2026
Signed-off-by: oliver könig <okoenig@nvidia.com>
Signed-off-by: sowmen <sowmendipta@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

r0.3.0 Cherry-pick label for r0.3.0 release branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants