chore: simplify some code inspired by RET505

[henryiii]

Simplifies some functions based on RET. I also moved one to a ternary.

[brettcannon]

While I do appreciate the simplifications, I personally am one of those people who still use elif even it the prior block has a return just in case things change in the future.

[henryiii]

So would you like me to leave (some?) simplifications, but remove the rule?

[henryiii]

Also, removing a return would be a pretty big change, usually restructuring an entire function. Do you have an example of how things could change where leaving an else would help?

I do like else if it is symmetric, but there was only one that was symmetric, and I solved that by switching it to a ternary. I think no else looks much better if the final match is a catch all, which many of these were.

[notatallshaw]

While I do appreciate the simplifications, I personally am one of those people who still use elif even it the prior block has a return just in case things change in the future.

To a weaker extent, I strongly prefer the style of adding a blank line in front of a new if block, to make it clear it is new block and now an elif in the old block, I think this makes it easier to read the control flow.

Also, removing a return would be a pretty big change, usually restructuring an entire function. Do you have an example of how things could change where leaving an else would help?

I would imagine something like this:

def foo(bar):
    if bar > 0:
        bar -= 1
        return bar
    elif bar < 0:
        bar += 2
        return bar
    return bar

To this is fine:

def foo(bar):
    if bar > 0:
        bar -= 1
    elif bar < 0:
        bar += 2
    return bar

But this:

def foo(bar):
    if bar > 0:
        bar -= 1
        return bar
    if bar < 0:
        bar += 2
        return bar
    return bar

To this is breaking:

def foo(bar):
    if bar > 0:
        bar -= 1
    if < 0:
        bar += 2
    return bar

That's why, in my personal style, I prefer a blank line when starting a new if block, I think it is easier to read:

def foo(bar):
    if bar > 0:
        bar -= 1
        return bar
    
    if bar < 0:
        bar += 2
        return bar

    return bar

[henryiii]

I've removed the rule 505 (though 506-509 are still enabled - I can disable them too if desired), and added spaces after many of the returns. I should note there are lots of examples of if-<control-flow> without else's already here.

[henryiii]

I don't think you can remove a control flow statement and expect not to touch or look at anything around it; I think simpler to read is better than trying to make a structure that requires less changing to modify.

That example does not break, by the way.

def foo(bar):
    if bar > 0:
        bar -= 1
    if bar < 0:
        bar += 2
    return bar

returns exactly the same thing as

def foo(bar):
    if bar > 0:
        bar -= 1
        return bar
    if bar < 0:
        bar += 2
        return bar
    return bar

If the conditions don't overlap, then it would be breaking, as something could trigger both. But I still think in practice the simplicity of avoiding the else's (else's dangle) is better.

The place I think it's better is if you have symmetry:

def foo(bar):
    if bar:
        return 1
    else:
        return 2

But you can almost always work around it (just like you'd work around a formatter), for example, if there are only two options, this often works:

def foo(bar):
    return 1 if bar else 2

Pattern matching also works well for things with symmetry. Etc.

[notatallshaw]

I don't think you can remove a control flow statement and expect not to touch or look at anything around it; I think simpler to read is better than trying to make a structure that requires less changing to modify.

That example does not break, by the way.

def foo(bar):
    if bar > 0:
        bar -= 1
    if bar < 0:
        bar += 2
    return bar

returns exactly the same thing as

def foo(bar):
    if bar > 0:
        bar -= 1
        return bar
    if bar < 0:
        bar += 2
        return bar
    return bar

I think this proves @brettcannon's point, the subtle difference between the elif refactor and the if refactor was difficult to spot, but they don't return the same:

>>> def foo(bar):
...     if bar > 0:
...         bar -= 1
...     if bar < 0:
...         bar += 2
...     return bar
...
>>> foo(0.5)
1.5

>>> def foo(bar):
...     if bar > 0:
...         bar -= 1
...         return bar
...     if bar < 0:
...         bar += 2
...         return bar
...     return bar
...
>>> foo(0.5)
-0.5

[henryiii]

Ahh, because it chains, interesting. But if you had this:

def foo(bar):
    if bar > 0:
        bar -= 1
        return bar
    elif bar < 0:  # noqa: RET505
        bar += 2
        return bar
    return bar

That would be even better, since you'd see that the decision to add an unused else here is intentional.

[brettcannon]

To be clear, I'm not going to block this going in, I'm also just not going to be the one that approves it either. 😉 But if someone else approves it then I think it's fine to go in.

[henryiii]

To be clear, the RET505 code is not enabled (and I can disable RET506-RET508 if preferred, too), this is now mostly just hand cleanups, and RET 501-504, which is things like mixing implicit and explicit returns.

[henryiii]

Oops, I put in on the test folder only, moved it to the general ignore, and added the other three (raise, continue, break) to ignore too.