Skip to content

[CodeStyle][W291] trim trailing whitespace in python file #45937

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Sep 14, 2022
Merged

[CodeStyle][W291] trim trailing whitespace in python file #45937

merged 4 commits into from
Sep 14, 2022

Conversation

SigureMo
Copy link
Member

@SigureMo SigureMo commented Sep 9, 2022 •
edited
Loading

PR types

Others

PR changes

Others

Describe

pip install docufix
docufix --trim-trailing-whitespace './**/*.py' './**/.*.py' --ignore-globs='./build/**/*.py' --fix

# check
pip install flake8
flake8 . --exclude=build/ --select=W291
# ✨ all check passed!

# import #44474
pre-commit run trailing-whitespace --all-files
# trim trailing whitespace.................................................Passed

本 PR merge 后应可 merge #44474

@SigureMo SigureMo changed the title [CodeStyle] trim trailing whitespace (W291) [WIP][CodeStyle] trim trailing whitespace (W291) Sep 9, 2022
@SigureMo SigureMo changed the title [WIP][CodeStyle] trim trailing whitespace (W291) [WIP][CodeStyle][W291] trim trailing whitespace Sep 9, 2022
@paddle-bot-old paddle-bot-old bot added the contributor External developers label Sep 9, 2022
@SigureMo SigureMo changed the title [WIP][CodeStyle][W291] trim trailing whitespace [CodeStyle][W291] trim trailing whitespace Sep 9, 2022
luotao1
luotao1 previously approved these changes Sep 13, 2022
View reviewed changes
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 13, 2022

https://xly.bce.baidu.com/paddlepaddle/paddle/newipipe/detail/6580153/job/18299804
static-check流水线发现很多代码示例错误。
和 @SigureMo 验证了：

  • 这些代码示例错误并不是由于本PR删除空格引起的，develop分支就会报错。如下面这个例子，就需要加上paddle.enable_static()

Paddle/python/paddle/fluid/layers/nn.py

Lines 6858 to 6865 in 9f5b083

Examples:
.. code-block:: python
import paddle.fluid as fluid
x = fluid.layers.data(name='x', shape=[10])
y = fluid.layers.data(name='y', shape=[10, 20], lod_level=2)
out = fluid.layers.lod_reset(x=x, y=y)
"""

image

  • 可能的原因是：我们触发API示例代码检查是改动了这些API的参数/文档时才会触发，有些改全局行为的代码不会触发static-check流水线。

请 @SigureMo 单独开一个issue，整理下这些问题，待文档修复小组看如何修改。

  • 其中大多数是fluid相关，非 fluid 10个左右。如上面的lod_reset API在官网没有展示。如果fluid即将废弃，不做修改也可以。

请 @Ligoml 审核下

@SigureMo SigureMo mentioned this pull request Sep 13, 2022
Closed
Ligoml
Ligoml previously approved these changes Sep 13, 2022
View reviewed changes
Copy link
Contributor

@Ligoml Ligoml left a comment

Choose a reason for hiding this comment

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

LGTM for docs

@SigureMo SigureMo changed the title [CodeStyle][W291] trim trailing whitespace [CodeStyle][W291] trim trailing whitespace in python file Sep 13, 2022
@luotao1
Copy link
Contributor

luotao1 commented Sep 13, 2022 •
edited
Loading

该PR仅修复了py文件的行末多余空格，可以在后续PR中修复其他类型文件的行末多余空格。
虽然涉及文件类型较多
image
和 @SigureMo 讨论，仅修改以下几类：

  • md、rst（只有一个 README）
  • C++： .h .cc .cu .cuh .cpp
  • CMake：.cmake CMakeLists.txt

原因是：

  • 当前的格式检查仅针对C++/C/Cuda/python/CMake文件，这几类文件占了99%

image

  • Dockerfile/shell脚本等的检查，CI无保证，修改风险较大。

raindrops2sea
raindrops2sea previously approved these changes Sep 14, 2022
View reviewed changes
@luotao1
Copy link
Contributor

luotao1 commented Sep 14, 2022

@SigureMo 请解决下冲突

@SigureMo SigureMo dismissed stale reviews from raindrops2sea, Ligoml, and luotao1 via 48adf73 September 14, 2022 06:59
@SigureMo SigureMo mentioned this pull request Sep 14, 2022
Closed
@SigureMo SigureMo mentioned this pull request Sep 14, 2022
Merged
luotao1
luotao1 approved these changes Sep 14, 2022
View reviewed changes
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 luotao1 merged commit de8c0ba into PaddlePaddle:develop Sep 14, 2022
@SigureMo SigureMo deleted the trim-trailing-whitespace branch September 14, 2022 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@luotao1 luotao1 luotao1 approved these changes

@raindrops2sea raindrops2sea raindrops2sea left review comments

@Ligoml Ligoml Ligoml left review comments

Assignees

@luotao1 luotao1

@Ligoml Ligoml

Labels
contributor External developers
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@SigureMo @luotao1 @raindrops2sea @Ligoml