-
Notifications
You must be signed in to change notification settings - Fork 30.9k
Fix fix_and_overwrite mode of utils/check_docstring.py
#39369
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 fix_and_overwrite mode of utils/check_docstring.py
#39369
Conversation
558ffe1 to
0ac013c
Compare
0ac013c to
865312b
Compare
|
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. |
|
I won't be available to review until next Friday. BTW, do you mean the issue you found for |
|
Yes, that one. Enjoy!! Its minor, it can wait for a week :) |
utils/check_docstring.pyfix_and_overwrite mode of utils/check_docstring.py
|
@ydshieh gentle ping on this 🤗 Btw, I have successfully fixed a few docstrings using the new version :) |
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 diving into this!
One question:
obj.doc attribute. Python automatically un-indents the string provided by doc
Is this always? Because if so, we should have troubles at many places (before this PR) no? But we only have seen a few places.
It only affects the fixing mode, not the checks on the docstring. I guess devs have got used to fixing the docs manually (we can ask). Every time I had to fix a doc issue in my cache refactor this issue happened, and I had to either cherry pick this or fix the docstring manually. Also, If a docstring is placed at root level indentation, there is no difference. But it is indeed a consistent behaviour: |
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.
Thank you. Makes sense to me. Guess you are right, this is likely not running in fix mode for long time
Fixed it @ydshieh :)
The script was failing to apply fixes because of a mismatch in how it reads the docstring for comparison versus how it reads it for replacement.
obj.__doc__attribute. Python automatically un-indents the string provided by__doc__.inspect.getsourcelines(), which preserves the original indentation.The
fix_docstringfunction would first try to locate the argument block within the raw source code. To ensure it had found the right place, it would compare the text from the source (with indentation) against the text from__doc__(without indentation). This comparison would then fail.Probably the check we talked about is not needed anymore, maybe should we remove it? Or leave it as-is to be extra cautious?