Skip to content

Conversation

Skylion007
Copy link
Contributor

Enable ruff rule FURB188 to str remove(pref|suf)fix - more readable and efficient.

Copy link
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

django-stubs (https://github.com/typeddjango/django-stubs): 2.02x faster (51.2s -> 25.4s in a single noisy sample)

freqtrade (https://github.com/freqtrade/freqtrade): 1.73x slower (130.8s -> 225.8s in a single noisy sample)

@Skylion007 Skylion007 changed the title Enables ruff FURB188: str.remove(pre|suf)fix Enable ruff FURB188: str.remove(pre|suf)fix Feb 13, 2025
Copy link
Collaborator

@cdce8p cdce8p left a comment

Choose a reason for hiding this comment

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

more readable and efficient.

Readable yes. Efficient is a different story. Mypyc can optimize slicing, whereas removesuffix isn't optimized (yet).

It probably doesn't make a huge difference here, I'd be careful though to generalize it and adding the ruff rule. The same arguments apply to many other ruff rules as well btw.

Copy link
Collaborator

@hauntsaninja hauntsaninja left a comment

Choose a reason for hiding this comment

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

Thanks! And thanks to Marc for teaching mypyc about remove* methods in #18672

@hauntsaninja hauntsaninja merged commit 763185d into python:master Feb 14, 2025
18 checks passed
x612skm pushed a commit to x612skm/mypy-dev that referenced this pull request Feb 24, 2025
@sterliakov
Copy link
Collaborator

@Skylion007 does this need upgrading from 0.8.6 (current version in pre-commit) to latest 0.9.x ruff? Now pre-commit emits a warning (only in verbose mode, it isn't an error and doesn't block the commit), and the rule doesn't seem to be actually enforced:

$ pre-commit run -a -v
...
ruff.....................................................................Passed
- hook id: ruff
- duration: 0.16s

warning: Selection `FURB188` has no effect because preview is not enabled.
All checks passed!
...

The rule left preview in 0.9.0.

@cdce8p
Copy link
Collaborator

cdce8p commented Mar 11, 2025

does this need upgrading from 0.8.6 (current version in pre-commit) to latest 0.9.x ruff? Now pre-commit emits a warning (only in verbose mode, it isn't an error and doesn't block the commit), and the rule doesn't seem to be actually enforced

Opened #18788 to update the ruff version to 0.9.10.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants