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

[CodeStyle][E401][F811] remove redefined imports and variables #48538

Merged
merged 11 commits into from
Dec 5, 2022

Conversation

SigureMo
Copy link
Member

@SigureMo SigureMo commented Nov 29, 2022

PR types

Others

PR changes

Others

Describe

本 PR 消除 E401(multiple imports on one line)和 F811(redefinition of unused name from line N)错误码

E401 就是多个 import 在一行(仅仅是 import 不包含 from import,后者推荐写在一起),拆开即可,这个本就极少,因此不单独作为一个 PR 了

F811 绝大多数是重复的 import,isort 引入过程中已经解决了绝大部分,剩下的根据情况手动解决,主要包含以下问题:

  • 因为 import API 路径 / 方式不同而未被 isort 自动合并的 -> 手动删除不合适的那一个
  • 单测重名问题 -> 根据情况修改为合适的名字即可,如非重复不删除单测,少数是重复的则删除,对于需要修改多一些的单测暂时在配置 ignore,在下一个 PR 进行修改
  • 少数 F811 错检或者不合适进行修改的 -> 加 noqa

Related links

@paddle-bot
Copy link

paddle-bot bot commented Nov 29, 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 Nov 29, 2022
@SigureMo SigureMo changed the title [CodeStyle][E401][F811] remove redefined imports and variables [WIP][CodeStyle][E401][F811] remove redefined imports and variables Nov 29, 2022
@luotao1 luotao1 self-assigned this Nov 30, 2022
@paddle-bot

This comment was marked as off-topic.

Copy link
Member Author

@SigureMo SigureMo left a comment

Choose a reason for hiding this comment

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

@luotao1 这个 PR 可以开始 review 啦~

@@ -71,7 +71,7 @@ def test_sub_scalar(self):
np.testing.assert_allclose(a_np - 10, b_np, rtol=1e-05)

@prog_scope()
def test_radd_scalar(self):
def test_rsub_scalar(self):
Copy link
Member Author

Choose a reason for hiding this comment

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

这里测试的是 __rsub__ 但和上面 __radd__ 重名了,因此修改前 test_radd_scalar 其实没有跑,里面有一个忘记 unpack 的 b_np,就 unpack 了下~

@@ -184,7 +184,7 @@ def func_test_create_param_lr_with_no_1_value_for_coverage(self):
z.backward()
opt.step()

def func_test_create_param_lr_with_no_1_value_for_coverage(self):
def test_create_param_lr_with_no_1_value_for_coverage(self):
Copy link
Member Author

Choose a reason for hiding this comment

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

明显的命名错误,导致这里没有被测试

with _test_eager_guard():
self.func_tensor_str_bf16()
self.func_tensor_str_bf16()

Copy link
Member Author

Choose a reason for hiding this comment

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

和上面 1378-1381 完全一致,因此移除

@@ -117,7 +117,7 @@ def _elementwise_mul_flops(input_shapes, attrs):


@register_flops("elementwise_div")
def _elementwise_mul_flops(input_shapes, attrs):
def _elementwise_div_flops(input_shapes, attrs):
Copy link
Member Author

Choose a reason for hiding this comment

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

明显的命名错误

python/paddle/fluid/tests/unittests/test_activation_nn_grad.py: F811
python/paddle/fluid/tests/unittests/test_lstm_cudnn_op.py: F811
python/paddle/fluid/tests/unittests/test_matmul_v2_op.py: F811
python/paddle/fluid/tests/unittests/test_rrelu_op.py: F811
Copy link
Member Author

@SigureMo SigureMo Dec 1, 2022

Choose a reason for hiding this comment

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

这五个单测 F811 问题导致隐藏了一些错误,修复时暴露了这些问题并需要修复下,可能需要修改下单测,因此拆分到之后 PR 处理

这个 PR 里处理的都是一些显而易见的错误~

@SigureMo SigureMo changed the title [WIP][CodeStyle][E401][F811] remove redefined imports and variables [CodeStyle][E401][F811] remove redefined imports and variables Dec 1, 2022
Copy link
Contributor

@XiaoguangHu01 XiaoguangHu01 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 luotao1 merged commit 89bd401 into PaddlePaddle:develop Dec 5, 2022
@SigureMo SigureMo deleted the E401/F811/fix branch December 5, 2022 10:21
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.

[F811] redefinition of unused name from line N [E401] multiple imports on one line
3 participants