astral-sh / ruff

An extremely fast Python linter and code formatter, written in Rust.
https://docs.astral.sh/ruff
MIT License
33.23k stars 1.11k forks source link

Rule idea: dict get for ranking dict #14592

Closed entorb closed 3 days ago

entorb commented 3 days ago

Do you think a rule for improving code of ranking-dict increment would be worth implementing?

this

if "key" not in d:
    d["key"] = 1
else:
    d["key"] += 1

should be simplified by

d["key"] = d.get("key", 1)
dylwil3 commented 3 days ago

Thanks for suggesting a new lint rule! It's always nice to have simplification ideas.

In this case I'm having a little trouble understanding the suggestion. Are the two code blocks really equivalent? It seems like the latter does not actually add 1 to the existing entry. I guess this would work:

d["key"] = d.get("key",0) + 1

But, at least to me, that takes a little more time to understand than the if/else statement. I would think that the latter is also slightly slower (but I could be wrong, and that's not that big of a deal anyway).

Apologies if I've misunderstood the suggestion!

entorb commented 3 days ago

Ups, yes of cause you are right, as I messed the second part up. Sorry for the confusion. Here corrected:

this

if "key" not in d:
    d["key"] = 1
else:
    d["key"] += 1

should be simplified by

d["key"] = d.get("key", 0) + 1
dylwil3 commented 3 days ago

Great, thank you! I think, as I said above, that I would personally find this lint confusing. But of course I'm happy to be overruled by the community 😄

MichaReiser commented 3 days ago

Thanks for the suggestion

One acceptance criteria for our rules is that the rule should be useful for most Python programmers and that there's enough support in the community to consider it good practice:

Generally, if a lint is triggered, this should be useful to most Python (3+) programmers seeing it most of the time.

I don't think there's enough evidence for avoiding the if...else branching in the above example to be considered "simpler".