Add docstring checker. #9848
Add docstring checker. #9848
Conversation
tools/codestyle/docstring_checker.py
Outdated
| """ | ||
|
|
||
| def __init__(self): | ||
| from collections import defaultdict |
There was a problem hiding this comment.
Can you move import to the top?
| self.d['Raises'] = [] | ||
| self.args = {} #arg_name->arg_type | ||
|
|
||
| def get_level(self, string, indent=' '): |
There was a problem hiding this comment.
Have we agreed that the indent is 4, instead of 2?
There was a problem hiding this comment.
Come from google python style guide.
And now almost all of the comments use 4 spaces indention.
|
|
||
| level = self.get_level(l) | ||
| if c.startswith("Args:"): | ||
| state = ("Args", level) |
There was a problem hiding this comment.
I think we need to check with our auto-doc generator and see it can parse the new style
There was a problem hiding this comment.
在我们的文档里边其实规定了参数的风格：https://github.com/PaddlePaddle/Paddle/blob/develop/doc/fluid/dev/api_doc_std_cn.md。
而且现在绝大多数都是按照这种风格来的。
tools/codestyle/docstring_checker.py
Outdated
| 'W9003': ('All args with their types must be mentioned in doc string', | ||
| symbol + "-with-all-args", | ||
| 'Used when not all arguments are in the doc string '), | ||
| 'W9005': ('Missing docstring or docstring is too shorter', |
|
|
||
| #exit $TOTAL_ERRORS | ||
| #For now, just warning: | ||
| exit 0 |
There was a problem hiding this comment.
It will be great that pylint only requires the author to polish the code style for the codes he/she has touched, instead of covering the whole file.
There was a problem hiding this comment.
It's a great idea, but maybe we need consistency in the whole Python file? E.g., the old file is 2-space indentation, the rule could be 4-space indentation, we probably want the whole file to have the same 4-space indentation, rather than the modified lines. Some edge case is hard to handle as well, for example the developer changed the second line of a doc string, does the linter have to modify the first line as well?
| done | ||
|
|
||
| #exit $TOTAL_ERRORS | ||
| #For now, just warning: |
There was a problem hiding this comment.
Maybe we don't turn on it in CI, but fails locally? (user can still use git commit --no-verify to skip the check.
There was a problem hiding this comment.
我想了一下，是不是给使用者一个一致的认知比较好？
There was a problem hiding this comment.
我担心如果本地也不报失败，没有人会去花时间修复问题，理论上CI需要强制验证，不过可能我们可以有一个试运行期，本地报错，CI不强制认证。
|
|
||
| #exit $TOTAL_ERRORS | ||
| #For now, just warning: | ||
| exit 0 |
There was a problem hiding this comment.
It's a great idea, but maybe we need consistency in the whole Python file? E.g., the old file is 2-space indentation, the rule could be 4-space indentation, we probably want the whole file to have the same 4-space indentation, rather than the modified lines. Some edge case is hard to handle as well, for example the developer changed the second line of a doc string, does the linter have to modify the first line as well?
|
|
||
| return True | ||
|
|
||
| def all_args_in_doc(self, node, doc): |
There was a problem hiding this comment.
Let's talk offline. I'm excited that you can get args from a python function. We can use this solution to detect API change and enforce API-compatibility.
|
@shanyi15 can you talk with @gongweibao and see if the current python comment requirement can satisfy the API documentation on the website? |
|
@panyx0718 Sure |
Fix #9778 (comment)