Skip to content

Refactor CLI [9/N]: Replace HfArgumentParser from transformers with local#5210

Merged
albertvillanova merged 8 commits intohuggingface:mainfrom
albertvillanova:refactor-cli-9
Mar 5, 2026
Merged

Refactor CLI [9/N]: Replace HfArgumentParser from transformers with local#5210
albertvillanova merged 8 commits intohuggingface:mainfrom
albertvillanova:refactor-cli-9

Conversation

@albertvillanova
Copy link
Copy Markdown
Member

@albertvillanova albertvillanova commented Mar 3, 2026

Replace HfArgumentParser from transformers with local.

This PR updates the import statements in trl/scripts/utils.py to use local versions of argument parsing utilities instead of those from the external transformers library. This change helps improving latency within the codebase.

The latency of import trl.scripts.utils changes from 7.5s to <0.5s after the merge of:

See upstream issue:

Dependency management:

  • Replaced imports of HfArgumentParser, DataClass, and DataClassType from the transformers library with imports from the local trl.scripts._hf_argparser module.

Note

Medium Risk
Although intended as a drop-in copy, vendoring core CLI argument parsing code can subtly change parsing behavior and may drift from upstream fixes, impacting script configuration/flag handling.

Overview
Introduces a vendored copy of transformershf_argparser as trl/scripts/_hf_argparser.py, with yaml imported lazily in parse_yaml_file to avoid the upstream import-time latency.

Updates trl/scripts/utils.py to import HfArgumentParser/DataClass/DataClassType from the local module instead of transformers, keeping CLI parsing behavior local and reducing startup overhead.

Written by Cursor Bugbot for commit 2e15b40. This will update automatically on new commits. Configure here.

@qgallouedec
Copy link
Copy Markdown
Member

does it mean that when huggingface/transformers#44273 is solved, we could just revert this commit?

@albertvillanova
Copy link
Copy Markdown
Member Author

Thanks for your review, @qgallouedec.

Not exactly, because we still support older versions of transformers where the import latency is not fixed.

Instead, we could make a conditional import depending on the transformers version.
What do you think?

@qgallouedec
Copy link
Copy Markdown
Member

I'd recommend at least a comment explaining that it's copied from transformers and modified because of the latency (you already did it) +

up to you

@albertvillanova albertvillanova merged commit a1488dd into huggingface:main Mar 5, 2026
12 checks passed
catherinelee274 pushed a commit to catherinelee274/trl that referenced this pull request Apr 17, 2026
songhappy pushed a commit to songhappy/trl that referenced this pull request Apr 20, 2026
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