Skip to content

Conversation

SigureMo
Copy link
Member

@SigureMo SigureMo commented Oct 28, 2022

PR types

Others

PR changes

Others

Describe

解决 Flake8 E711 问题(comparison to None should be ‘if cond is None:’)

  • x == None -> x is None
  • x != None -> x is not None

并调整单测中的相关使用(与 None 相关的比较),如 self.assertTrue(x is None) -> self.assertIsNone(x),可以优化报错信息:

import unittest

class Test1(unittest.TestCase):
    def test(self):
        x = 1
        self.assertTrue(x is None)

# 不会说明 x 到底是什么值
Test1().test() # AssertionError: False is not true

class Test2(unittest.TestCase):
    def test(self):
        x = 1
        self.assertIsNone(x)

# 会同时说明 x 是什么
Test2().test() # AssertionError: 1 is not None

全部替换如下:

  • self.assertTrue(x is None) -> self.assertIsNone(x)
  • self.assertTrue(x is not None) -> self.assertIsNotNone(x)
  • self.assertFalse(x is None) -> self.assertIsNotNone(x)
  • self.assertFalse(x is not None) -> self.assertIsNone(x)(无此情况)
  • self.assertEqual(x, None) -> self.assertIsNone(x)
  • self.assertNotEqual(x, None) -> self.assertIsNotNone(x)

具体所使用工具及命令如下:

# ast-grep, docs: https://ast-grep.github.io/
# clone ast-grep to local
git clone https://github.com/ast-grep/ast-grep.git
cd ast-grep
# run ast-grep in dev mode
cargo run -- --pattern '$COND == None' --rewrite '$COND is None' --lang py -i /path/to/paddle
cargo run -- --pattern '$COND != None' --rewrite '$COND is not None' --lang py -i /path/to/paddle
# ...

Related links

@paddle-bot
Copy link

paddle-bot bot commented Oct 28, 2022

你的PR提交成功,感谢你对开源项目的贡献!
请关注后续CI自动化测试结果,详情请参考Paddle-CI手册
Your PR has been submitted. Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

@paddle-bot paddle-bot bot added contributor External developers status: proposed labels Oct 28, 2022
@SigureMo SigureMo changed the title [WIP][CodeStyle][E711] use is/is not for comparison with None [CodeStyle][E711] use is/is not for comparison with None Oct 28, 2022
@luotao1 luotao1 self-assigned this Oct 31, 2022
@luotao1
Copy link
Contributor

luotao1 commented Oct 31, 2022

ast-grep, docs: https://ast-grep.github.io/

是否考虑将修复工具引入precommit配置?

@SigureMo
Copy link
Member Author

SigureMo commented Oct 31, 2022

是否考虑将修复工具引入precommit配置?

ast-grep 的话暂时不考虑,一方面它并不是 python 生态的,而是 Rust / Node.js 生态的工具,目前不方便集成,另一方面它刚刚开发几个月,非常不成熟

luotao1
luotao1 previously approved these changes Nov 1, 2022
return _legacy_C_ops.reduce_any(
x, 'dim', axis, 'keep_dim', keepdim, 'reduce_all', reduce_all_flag
)

attrs = {
'dim': axis if axis != None and axis != [] and axis != () else [0],
'dim': axis if axis is not None and axis != [] and axis != () else [0],
'keep_dim': keepdim,
'reduce_all': reduce_all_flag,
}
Copy link
Contributor

@luotao1 luotao1 Nov 1, 2022

Choose a reason for hiding this comment

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

2022-10-31 18:16:21 0. APIs without core.ops: 
2022-10-31 18:16:21 paddle.fluid.layers.nn.reduce_any
2022-10-31 18:16:21 You must have one RD (JiabinYang (Recommend) or wanghuancoder, phlrain) approval for the api change for the opreator-related api without '_C_ops'.
2022-10-31 18:16:21 For more details, please click [https://github.com/PaddlePaddle/Paddle/wiki/paddle_api_development_manual.md]

@wanghuancoder @JiabinYang review

@luotao1
Copy link
Contributor

luotao1 commented Nov 1, 2022

ast-grep 的话暂时不考虑,一方面它并不是 python 生态的,而是 Rust / Node.js 生态的工具,目前不方便集成,另一方面它刚刚开发几个月,非常不成熟

Got it. 因为本PR 【E711】和 【E712】#47464 的修改不能使用自动化,可以将两个描述中的修改方法写在wiki里,在增量拦截的时候 #47465 提示给开发者。

@SigureMo
Copy link
Member Author

SigureMo commented Nov 1, 2022

因为本PR 【E711】和 【E712】#47464 的修改不能使用自动化,可以将两个描述中的修改方法写在wiki里,在增量拦截的时候 #47465 提示给开发者。

是在 CI 使用 grep 匹配增量中的 x == None 这类 E711、E712 问题吗,不过按照正常流程的话(安装了 pre-commit),有这类问题的 commit 是无法通过 pre-commit 的,所以这样的问题不会到 CI 这一步,也就无法展示提示信息,除非开发者没有安装 pre-commit

另外就是 grep 无法通过 ast 分析代码,可能会造成一些误检

@luotao1
Copy link
Contributor

luotao1 commented Nov 1, 2022

有这类问题的 commit 是无法通过 pre-commit 的,所以这样的问题不会到 CI 这一步,也就无法展示提示信息,除非开发者没有安装 pre-commit

确实如此。后续可以在 代码风格检查指南 里加一下常见错误码(无法自动格式化) 的处理方式。

@luotao1 luotao1 requested a review from wanghuancoder November 1, 2022 07:35
wanghuancoder
wanghuancoder previously approved these changes Nov 1, 2022
Copy link
Contributor

@wanghuancoder wanghuancoder left a comment

Choose a reason for hiding this comment

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

LGTM

@luotao1
Copy link
Contributor

luotao1 commented Nov 1, 2022

请解决下冲突

@SigureMo SigureMo dismissed stale reviews from wanghuancoder and luotao1 via fb39d22 November 1, 2022 09:21
@SigureMo
Copy link
Member Author

SigureMo commented Nov 1, 2022

请解决下冲突

已解决~

@luotao1 luotao1 merged commit a35a4a5 into PaddlePaddle:develop Nov 1, 2022
@SigureMo SigureMo deleted the E711/fix branch November 1, 2022 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor External developers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[E711] comparison to None should be ‘if cond is None:’
3 participants