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][UP018] remove unnecessary call to str #51922

Merged
merged 1 commit into from
Mar 22, 2023

Conversation

jinyouzhi
Copy link
Contributor

@jinyouzhi jinyouzhi commented Mar 21, 2023

PR types

Others

PR changes

Others

Describe

UP018 rule 目的在于消除套在字符串字面值之上的额外 str() 调用,原始规则来自 https://github.com/asottile/pyupgrade#forced-strnative-literals

对存量代码利用 ruff --select UP018 . --fix 修复后逐一检查,共 51 处,可分为两类情况:

  1. str('xxxxx') -> 'xxxx'
  2. str() -> ''

均未引入语法歧义,且对可读性有所改善。故:

  • 是否可以引入本 rule:✅ 可引入,在当前 Python 版本中,字符串字面值类型即为 str 无需冗余调用,可改善可读性。
  • 是否可引入自动修复:✅ 可引入。

Related links

Copy link
Member

@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.

LGTM

@SigureMo
Copy link
Member

SigureMo commented Mar 21, 2023

UP018 rule 目的在于消除套在字符串字面值之上的额外 str() 调用,原始规则来自 asottile/pyupgrade#forced-strnative-literals

虽然 Paddle 目前存量仅包含 str 的情况,但是 ruff 实现的 pyupgrade 还包含 bytes 的字面量(虽然文档没写,但从文档描述 Unnecessary call to {literal_type},其中 {literal_type} 是个占位符可以猜到是有其他情况的),这个最好也要考虑到,该情况与 str 情况相同,同样可以引入 + 自动修复,因此没有问题

https://github.com/charliermarsh/ruff/blob/b5edc6dfc96e505c6b5079856166b5236fec9a87/crates/ruff/src/rules/pyupgrade/rules/native_literals.rs#L102

@paddle-bot paddle-bot bot added the contributor External developers label Mar 21, 2023
@jinyouzhi
Copy link
Contributor Author

@SigureMo CI跑完了

@luotao1 luotao1 merged commit 52a31b8 into PaddlePaddle:develop Mar 22, 2023
@jinyouzhi jinyouzhi deleted the style/up018 branch August 29, 2023 12: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.

3 participants