[Question]: Why is FURB131 good in this case?

[theRealProHacker]

Overview

I have the following code:

del d["encoding"]

Refurb tells me to change it to the following

d.pop("encoding")

I don't understand why I should use pop. IMO, pop and del have a very different meaning:

• pop means I want to use the value
• del means I want to delete the value but explicitly not use it

Could you maybe explain your reasoning?

Proposal

My proposal would be to remove 131. But you probably have a good reason, so this is rather a question than a proposal.

Apart from that, I would love a wiki or documentation that explains some more controversial design decisions.
I believe that is part of any linter/formatter/type checker, even if it requires a lot of effort. Maybe you could use the GitHub wiki (preferably) or GitHub Pages

[theRealProHacker]

Maybe a small addition: I do like list.clear() instead of del list[:]. That should definitely stay and is actually more expressive.

[dosisod]

When used on it's on (as a statement), del and pop are the same, though typically del is for when you don't care about the return value, whereas pop is for when you want to use it (as you mentioned). The reason I wrote this check is because if you don't know about pop, you could use del in ways that are un-pythonic (such as x = y["z"]; del y["z"]). Writing a check which reliably detects this is hard, and so it is easier to warn about all potential instances, and let the user decide what they want to do (though this means more potential false positives, which is unfortunate). Another reason to use pop over del that is not described in the docs is that it can take a default value, which might be useful if you are checking if a key exists before deleting it, and pop can also be used in a lambda/comprehension, whereas del cannot.

And yes, the del x[:] form is, IMO, much more readable, and should be separate from the normal del x[y] format. I have been playing with the idea of having "suffixes" for error codes, such that you could disable FURB131:pop but keep FURB131:clear enabled. This would allow you to keep the parts of the check that you like, while ignoring the parts you don't. Splitting the checks up is a last resort, since they share almost the same code, and I'd like to keep things as backwards compatible as possible.

Also, there are explanations for each of the checks, and you can access them via refurb --explain FURB131, or you can look at the online version as well. I try to provide as much context as I can for the reason the checks exist, and for what purpose they solve. It is hard to quantify "controversial" checks, since that means different things to different people, and I try to let the user decide what best suits them and their codebase. I will update the FURB131 docs to better reflect when del should be used instead of pop.

[theRealProHacker]

Just to start, this might seem offensive or aggressive, but it is not meant like this. I just sometimes have difficulties with writing "nicely"

To start

And yes, the del x[:] form is, IMO, much more readable, and should be separate from the normal del x[y] format.

Separating them might be a simple fix. Is it possible to add a new rule?

The reason I wrote this check is because if you don't know about pop, you could use del in ways that are un-pythonic (such as x = y["z"]; del y["z"]).

I fundamentally disagree with the idea that you should warn about something because it might, in rare instances, when the programmer is an absolute noob, lead to unpythonic code. It doesn't even catch bugs, apart from maybe some contrived cases.

If you want developers to know about pop in general, write an article (or a series of articles) with a title like "Things you should use, but don't know about yet". But this seems to be a bit of a sweeping blow.

Writing a check which reliably detects this is hard, and so it is easier to warn about all potential instances

It might be easier to just warn everyone, but the more I think about it, the more sure I am, that the better decision is to make that rule opt-in or deprecate it apart from the aforementioned exception ([:]). I don't want to disable this check every time I use refurb and others probably neither.

I would say that in most cases when someone is actually doing x = y["z"]; del y["z"], or something similar, he has a good reason.

Another reason to use pop over del that is not described in the docs is that it can take a default value, which might be useful if you are checking if a key exists before deleting it, and pop can also be used in a lambda/comprehension, whereas del cannot.

The both examples you named show why .pop() can be helpful over del (just like .get() has benefits over []), but in this context don't seem relevant. For the first one, what I said directly above applies. The second just doesn't make sense from what I understand.

Also, there are explanations for each of the checks, and you can access them via refurb --explain FURB131, or you can look at the online version as well.

I obviously looked at the explanation but find it pretty disturbing: For the first two cases, I would probably swap the good and bad example. And the reasoning is very vague: That you can use pop instead of del is a fact, but not a reason. I think if you had thought about the reasoning while writing the examples, you would have probably chosen different examples.

In essence, pop is not just generally better than del, but that's what the rule currently conveys (the name, the docs and the implementation).

I think the best solution might be to implement the "correct" intention. Which is x = y["z"]; del y["z"] and probably also what is mentioned in FURB132. I could try to make a PR if that's fine. There are some more difficult rules like 138 or 140.

[theRealProHacker]

This is more of a meta thing (so it's a separate comment):

I'd like to keep things as backwards compatible as possible

What does "backwards compatible" mean? Anyway, if you have a library like this, that has a lot of partially subjective, and a lot of unfinished or suboptimal rules (what I called controversial), you have to be able to make rapid changes for a good user experience IMO.