-
Notifications
You must be signed in to change notification settings - Fork 27.4k
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
Add missing tokenizer tests - Longformer #17677
Add missing tokenizer tests - Longformer #17677
Conversation
The documentation is not available anymore as the PR was closed or merged. |
I read discussion in merged tokenizers' tests PRs and post |
Thanks a lot for working on this @tgadeliya!! As far as I know, there are no identified "practices" for this case (cc @LysandreJik in case you have another opinion). Nevertheless, if changes are relevant, they are obviously welcome. For example, it is possible to indicate the changes made as here: transformers/src/transformers/models/deberta/modeling_deberta.py Lines 308 to 309 in d95a32c
If the differences are too long to list perhaps the message can just explain why it diverged from the originally copied and pasted code. Does this help you? |
@SaulLu, Sorry for the late reply. Summer is ending :) Thanks for your comment. Now it is clear for me. Actually, I came to the conclusion, that code cleaning not so necessary considering all pros and cons. So this PR can be reviewed and merged |
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.
Sounds great to me! Can you just merge/rebase on main so we can merge your PR?
@SaulLu I refreshed this PR, so now it is ready to merge |
Thanks @tgadeliya 🤗 |
What does this PR do?
This PR add tests for Longformer tokenizer copying tests from Roberta tokenizer's test suite, because those tokenizers are absolutely identical.
Fixes #16627
Before submitting
Pull Request section?
to it if that's the case.
documentation guidelines, and
here are tips on formatting docstrings.
Who can review?
@SaulLu @LysandreJik