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
Fix Sylvain's nits on the original KerasMetricCallback PR #18300
Conversation
The documentation is not available anymore as the PR was closed or 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.
Thanks for adding those!
src/transformers/keras_callbacks.py
Outdated
predict_with_generate (`bool`, *optional*, defaults to `False`): | ||
predict_with_generate (`bool`, defaults to `False`): | ||
Whether we should use `model.generate()` to get outputs for the model. | ||
use_xla_generation (`bool`, *optional*, defaults to `False`): |
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.
The optional should still be there in the docstring because the argument is optional in the sense it has a default value. It's just the type annotation Optional
(which means Union None
) that wasn't correct ;-)
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.
Ah, I never knew that was our rule for writing docstrings! Fixing.
Co-authored-by: Sylvain Gugger <35901082+sgugger@users.noreply.github.com>
…e#18300) * Fix Sylvain's nits on the original PR * Update src/transformers/keras_callbacks.py Co-authored-by: Sylvain Gugger <35901082+sgugger@users.noreply.github.com> * Re-add "optional" to docstring Co-authored-by: Sylvain Gugger <35901082+sgugger@users.noreply.github.com>
No description provided.