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] remove all future import #46411

Merged
merged 4 commits into from
Sep 27, 2022

Conversation

SigureMo
Copy link
Member

@SigureMo SigureMo commented Sep 22, 2022

PR types

Others

PR changes

Others

Describe

移除全部 from __future__ import xxx 结构,由于 Paddle 已经不支持 Python2.7 了,因此没有必要保留这些

future 详细说明见:https://docs.python.org/3/library/__future__.html

Paddle 目前代码里使用的全部是 3.0 及更高版本的内置特性(generator_stopannotations 未使用过),均无需从 __future__ import

# install remove-future-import
git clone https://github.com/cattidea/paddle-flake8-scripts.git
cd paddle-flake8-scripts
pip install .

# remove-future-import, remove the python2 `from __future__ import xxx` code
remove-future-import './**/*.py' './**/.*.*.py' --ignore-globs='./build/**/*.*,./build/**/.*.*,./**/*.pyc,./**/*.jpg,./**/*.graffle,./**/*.png' --fix

@paddle-bot
Copy link

paddle-bot bot commented Sep 22, 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 Sep 22, 2022
@luotao1 luotao1 self-assigned this Sep 22, 2022
@luotao1
Copy link
Contributor

luotao1 commented Sep 22, 2022

test_error (Failed),可以先revert 这个文件的修改

@SigureMo
Copy link
Member Author

SigureMo commented Sep 22, 2022

test_error (Failed),可以先revert 这个文件的修改

已 revert

该文件报错是因为其断言函数所使用的测试用例包含报错信息的位置,也就是依赖于测试的行号,移除 from __future__ import print_function 后整体行号 -1,因此依赖于行号的测试项将会报错

@luotao1
Copy link
Contributor

luotao1 commented Sep 23, 2022

TODO

移除 from future import print_function 后整体行号 -1,因此依赖于行号的测试项将会报错

  1. 可以单独提个PR修改下所有的行号

    def set_message(self):
    self.expected_message = \
    ['File "{}", line 35, in func_error_in_compile_time'.format(self.filepath),
    'inner_func()',
    'File "{}", line 28, in inner_func'.format(self.filepath),
    'def inner_func():',
    'fluid.layers.fill_constant(shape=[1, 2], value=9, dtype="int")',
    '<--- HERE',
    'return',
    ]

    同时可以对其使用yapf格式化
    - id: yapf
    files: (.*\.(py|bzl)|BUILD|.*\.BUILD|WORKSPACE)$
    exclude: |
    (?x)^(
    python/paddle/fluid/tests/unittests/dygraph_to_static/test_error.py|
    python/paddle/fluid/tests/unittests/dygraph_to_static/test_origin_info.py
    )$

  2. 可以在CI中对from __future__ import做监控,避免大家再使用了。

luotao1
luotao1 previously approved these changes Sep 23, 2022
Copy link
Contributor

@luotao1 luotao1 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 Sep 23, 2022

static-check 流水线报错,可以试下paddle.distributed.spawn的develop示例是否就报错

@SigureMo
Copy link
Member Author

SigureMo commented Sep 23, 2022

static-check 流水线报错,可以试下paddle.distributed.spawn的develop示例是否就报错

因为是分布式 API,所以我这边运行不了,要不这个先 revert?这个应该是我移除了这个示例代码里的 from __future__ import print_function 触发的

可以单独提个PR修改下所有的行号

这个我之后单独提 PR 做一下

可以在CI中对from __future__ import做监控,避免大家再使用了。

这个可以先排除掉 from __future__ import annotations,仅对其余的进行监控,因为如果未来支持 Type Hints 的话,annotations 将会非常有用(能在 Python3.7 使用最新版的类型提示语法)

@luotao1
Copy link
Contributor

luotao1 commented Sep 23, 2022

因为是分布式 API,所以我这边运行不了,要不这个先 revert?

可以的。后面验证后再单独提即可。

这个可以先排除掉 from future import annotations

可以的。

@SigureMo SigureMo changed the title [CodeStyle] remove all future import [CodeStyle][F404] remove all future import Sep 23, 2022
@SigureMo SigureMo changed the title [CodeStyle][F404] remove all future import [CodeStyle] remove all future import Sep 23, 2022
luotao1
luotao1 previously approved these changes Sep 23, 2022
Copy link
Contributor

@luotao1 luotao1 left a comment

Choose a reason for hiding this comment

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

LGTM

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.

3 participants