Skip to content
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 whitespace to multiline strings #13916

Merged
merged 1 commit into from
Oct 7, 2021

Conversation

aphedges
Copy link
Contributor

@aphedges aphedges commented Oct 7, 2021

What does this PR do?

When running a text generation experiment, I got the following error message:

Input length of input_ids is 104, but max_length is set to 100.This can lead to unexpected behavior. You should consider increasing config.max_length or max_length.

The error is missing a space before This can lead. Checking the source, the issue turned out to be that the string was wrapped across multiple lines, but there was no whitespace within the string itself to separate the two sentences. I searched for "\s+f?" throughout all of src/transformers, and this PR contains a fix for all similar whitespace errors I could find. The good news is that most multiline strings already handled whitespace properly, but there were still a lot that didn't. My search probably missed a few, but I did not feel like being exhaustive here.

I also made a couple of other error message fixes related to whitespace and punctuation, but focused on this specific whitespace issue.

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).

Who can review?

"Documentation" is the closest item to this PR, so @sgugger

Copy link
Collaborator

@sgugger sgugger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for all that work!

@sgugger sgugger merged commit 57420b1 into huggingface:master Oct 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants