[Aurelius84]

PR types

New features

PR changes

Others

Describe

What's new?

I found a great tool: autoflake, it has two import features for Paddle development:

• Remove unused-imports

• Remove unused-variables

[luotao1]

• @Aurelius84 [CodeStyle] Flake8 tracking issue #46039 中第16项 【F401】就是使用autoflake8来做格式化的。如 [CodeStyle][F401] remove unused imports in python/paddle/distributed #46758
• 请 @SigureMo 看下，autoflake8的配置是否要添加进来

[SigureMo]

原来不将 autoflake 直接添加到 pre-commit 是因为 autoflake 自动删除是有可能导致原本能跑通的代码删除后跑不通的，因此只让 flake8 提示而不自动删除，这样可以选择是加 # noqa: F401 还是手动删除掉，考虑到存量已经修复，所以这个工作量应该不大，如果自动修复的影响认为是可接受的我觉得可以引入

由于 unused variable 存量没清，所以还不能加进来（大概试了下，应该有 500+ 文件需要清理），这个可以等存量清理后再加上

请 @SigureMo 看下，autoflake8的配置是否要添加进来

配置的话貌似 autoflake 有点 bug，无法禁用掉 remove-unused-variables，所以暂时直接在 pre-commit-config 里 args 修改下就好：

-   repo: https://github.com/PyCQA/autoflake
    rev: v1.7.7
    hooks:
    -   id: autoflake
        args:
            - --in-place
            - --remove-all-unused-imports
            - --ignore-pass-after-docstring
            - --ignore-init-module-imports
            - --exclude=python/paddle/fluid/[!t]**,python/paddle/fluid/tra**

这里 fluid 下非单测需要 exclude 掉，因为没有清理存量

[Aurelius84]

-   repo: https://github.com/PyCQA/autoflake
    rev: v1.7.7
    hooks:
    -   id: autoflake
        args:
            - --in-place
            - --remove-all-unused-imports
            - --ignore-pass-after-docstring
            - --ignore-init-module-imports
            - --exclude=python/paddle/fluid/[!t]**,python/paddle/fluid/tra**

@SigureMo 我按照你上面这样修改，重新commit下？

[SigureMo]

@SigureMo 我按照你上面这样修改，重新commit下？

嗯嗯，仅修改配置的话可以加一下 test=document_fix

[Aurelius84]

exclude=python/paddle/fluid/[!t]**,python/paddle/fluid/tra**

@SigureMo 这一行的意思是仅仅对python/paddle/fluid 的 test 目录文件生效？

[SigureMo]

@SigureMo 这一行的意思是仅仅对python/paddle/fluid 的 test 目录文件生效？

嗯嗯，对的，同时是和 flake8 配置保持一致

Paddle/.flake8

Lines 5 to 8 in 60e0c50

	# A trick to exclude fluid/ but keep fluid/tests/, see more at
	# https://github.com/PaddlePaddle/Paddle/pull/46290#discussion_r976392010
	./python/paddle/fluid/[!t]**,
	./python/paddle/fluid/tra**,

[Aurelius84]

好的，非常感谢 @SigureMo 同学解答 ✿✿ヽ(°▽°)ノ✿

[luotao1]

LGTM，也请 @SigureMo 再review下

无法禁用掉 remove-unused-variables

@SigureMo 这个是哪个错误码呢？

[luotao1]

因此只让 flake8 提示而不自动删除，这样可以选择是加 # noqa: F401 还是手动删除掉，考虑到存量已经修复，所以这个工作量应该不大，如果自动修复的影响认为是可接受的我觉得可以引入

如果加了autoflake8，# noqa: F401还生效么？因为当前有1700多处豁免的部分。 @SigureMo

[SigureMo]

@SigureMo 这个是哪个错误码呢？

unused variable 是错误码 F841（local variable name is assigned to but never used），存量 2124 处，需要之后考虑修复方案

如果加了autoflake8，# noqa: F401还生效么？因为当前有1700多处豁免的部分。 @SigureMo

autoflake 遵守 noqa 的规则，所以是生效的，可以通过运行 pre-commit run autoflake --all-files 来测试对全部文件的影响（除去 exclude 的），目前的话会将 python/paddle/fluid/tests/unittests/mkldnn/test_onnx_format_quantization_mobilenetv1.py 一个 pass 删除，这个修改是没有任何问题的，此外无任何其他影响

[SigureMo]

LGTM～