-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
chang check_op_desc.py to py3 #32825
Conversation
Thanks for your contribution! |
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.
如批注
tools/check_op_desc.py
Outdated
@@ -71,7 +72,8 @@ def diff_vars(origin_vars, new_vars): | |||
vars_name_only_in_new = set(new_vars.keys()) - set(origin_vars.keys()) | |||
|
|||
for var_name in common_vars_name: | |||
if cmp(origin_vars.get(var_name), new_vars.get(var_name)) == SAME: | |||
if operator.eq(origin_vars.get(var_name), |
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.
不要与SAME
比较了。
直接使用eq
的布尔值结果。
这个var_name
看着就想是字符串, 何不直接origin_vars.get(var_name) == new_vars.get(var_name)
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.
直接用布尔值做if,连==都可以省略,觉得如何?
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.
@liym27 觉得如何?
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.
LGTM
PR types
Others
PR changes
Others
Describe
在本地使用新的check_op_desc.py脚本做了测试