Accept processor in get_training_chat_template#5560
Accept processor in get_training_chat_template#5560qgallouedec wants to merge 21 commits intomainfrom
get_training_chat_template#5560Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aa61a92ea2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 6700975. Configure here.
Co-authored-by: Quentin Gallouédec <45557362+qgallouedec@users.noreply.github.com>
albertvillanova
left a comment
albertvillanova
left a comment
There was a problem hiding this comment.
CI is red:
AttributeError: 'NoneType' object has no attribute 'chat_template'
3 participants
get_training_chat_templatealready works on bothPreTrainedTokenizerandProcessorMixin. The type hint, parameter name, and docstring were misleading by saying "tokenizer" only. Call sites (e.g.GRPOTrainer,SFTTrainer) already passprocessing_class.Note
Low Risk
Low risk API tweak limited to
get_training_chat_template, adding a backward-compatible alias with a deprecation warning and clearer error handling. Behavior should be unchanged for existing callers that already pass a processor/tokenizer viaprocessing_class.Overview
Updates
get_training_chat_templateto formally accept either aPreTrainedTokenizerBaseor aProcessorMixinvia a newprocessing_classparameter, aligning the signature and docs with how it’s already used.Adds a deprecated
tokenizeralias for backward compatibility (with aFutureWarningand a guard against passing both args), and switches internal checks to referenceprocessing_classplus a slightly generalized error message when patching isn’t supported.Reviewed by Cursor Bugbot for commit c001dbc. Bugbot is set up for automated code reviews on this repo. Configure here.