-
Notifications
You must be signed in to change notification settings - Fork 26.9k
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.
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
Enable padding_side
as call time kwargs
#33385
Conversation
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. |
The failing test is not related to this PR, seems to be related to training datasets and tasks. PR is ready for review! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome - thanks for adding!
Looks good to me thanks @zucchini-nlp, this should unblock a lot of the kwargs uniformization PRs :). Anything blocking before this gets merged? Is it because of the tests failing? |
I wanted to get @ArthurZucker 's review as well as this is tokenizers related. For tests I can rerun them and hope they won't fail this time |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great to me!
Thanks, can someone merge pls as the tests won't pass after rerunning? |
@zucchini-nlp Yep - I can merge |
* fix * add padding-side kwarg * add padding side in all models & fix tests * fix copies * fix tests
* fix * add padding-side kwarg * add padding side in all models & fix tests * fix copies * fix tests
What does this PR do?
This PR adds
padding_side
as a valid kwargs when calling tokenizers, so that the users can set padding side when tokenizing. Is a follow-up from #32858 (comment), where we found that most processors acceptpadding_side
but don't use it when tokenizing.Added test for that and ran it for all models