Remove part of FURB131: #239
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This PR removes a portion of FURB131 which suggested replacing most instances of `del` with `.pop()`. The reasoning behid it was not well founded, and in most cases, it is better to just leave the `del` statememt as it is. In one instance though, which is the `del x[:]` usage, it is more preferable to use the `.clear()` method, as it is faster and more readable. Closes #234.
dosisod
added a commit
that referenced
this pull request
Feb 1, 2024
FURB131 (previously `no-del`) has been broken for a while, basically since it's initial creation. #239 fixed some of this, but introduced improper semantics by implying you could use `del x[:]` when `x` is a `dict`, which is not true. This means that FURB131 only really applies to `list` types now (or any sequence that implements `.clear()`, though this isn't supported yet). I recently came across a similar idiom for clearing an array, `x[:] = []`, which is about twice as slow as `x.clear()`, and similar in nature to `del x[:]`. This new update was what prompted me to fix all the broken logic. With that in mind, this check has been renamed to `use-clear` since it no longer is about the `del` statement, but about using slices to clear a list. This shouldn't cause any issues since the name of checks are only used for display purposes. This will break any hyperlinks to FURB131 in the docs, though you'll still end up on the same page so I'm not too concerned about that.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR removes a portion of FURB131 which suggested replacing most instances of
delwith.pop(). The reasoning behid it was not well founded, and in most cases, it is better to just leave thedelstatememt as it is. In one instance though, which is thedel x[:]usage, it is more preferable to use the.clear()method, as it is faster and more readable.Closes #234.