-
Notifications
You must be signed in to change notification settings - Fork 27.3k
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 warning to let the user know that the __call__
method is faster than encode
+ pad
for a fast tokenizer
#18693
Conversation
__call__
method is fast than encode
+ pad
for a fast tokenizer__call__
method is faster than encode
+ pad
for a fast tokenizer
@@ -2869,6 +2872,12 @@ def pad( | |||
verbose (`bool`, *optional*, defaults to `True`): | |||
Whether or not to print more information and warnings. | |||
""" | |||
if self.__class__.__name__.endswith("Fast"): | |||
warnings.warn( |
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.
I use warnings.warn
instead of logger.warning
so that the warning can be issued only once.
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. |
__call__
method is faster than encode
+ pad
for a fast tokenizer__call__
method is faster than encode
+ pad
for a fast tokenizer
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.
Thanks for your PR! I'd rather we use logging.warning
instead of warnings.warn
as the former can be managed by the centralized logging system.
We have a system in place to only send these once, see how it's implemented in the base tokenization file:
transformers/src/transformers/tokenization_utils_base.py
Lines 1498 to 1500 in 4386980
self.deprecation_warnings = ( | |
{} | |
) # Use to store when we have already noticed a deprecation warning (avoid overlogging). |
I would implement something similar here :)
Thanks a lot for the advice @LysandreJik 🤗 ! Indeed, since this attribute |
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.
Great! Thanks for your PR, @SaulLu. Merging!
… than `encode` + `pad` for a fast tokenizer (huggingface#18693) * add warning to let the user know that the method is slower that for a fast tokenizer * user warnings * fix layoutlmv2 * fix layout* * change warnings into logger.warning
What does this PR do?
In this PR I propose to specify in the docstring of the
pad
method and to log a warning when the pad method is called with a fast tokenizer. The goal is to encourage the user to use the__call__
method to encode and pad their input at the same time since it will be faster then encoding and then padding it with the methodpad
(because it is not using the rust backend).Before submitting
Pull Request section?
to it if that's the case.
documentation guidelines, and
here are tips on formatting docstrings.
Who can review?
Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.