autopep8 icon indicating copy to clipboard operation
autopep8 copied to clipboard

if 'var == True:' being replaced with 'if var:'

Open Flamefire opened this issue 4 years ago • 4 comments

New issue after #86

To make it short: Even though it is done only in aggressive mode 2 it is wrong.

if var == True has the semantic of if var is True. E.g. [1] == True evaluates to False. So this "fix" changes semantic.

I deeply believe this should not be done. Even though it might be unintentional there are cases where an == True or better is True are wanted. Changing the semantic isn't something an autoformatter/autofixer should do.

Rationale: Core description is "autopep8 automatically formats Python code to conform to the PEP 8 style guide.". It is NOT up to the tool to "fix" possible coding errors only to make sure it is PEP8 correct

Flamefire avatar Apr 17 '20 09:04 Flamefire

Think if var == True should be changed to if var is True ;)

jedie avatar Jul 05 '20 19:07 jedie

I deeply believe this should not be done.

I totally agree, this change introduced an actual bug into our codebase - which a formatter should never do.

Our code (using Pandas dataframe) was:

if df.loc[index].flag == True:

which was changed to (using --in-place --aggressive --aggressive --experimental):

if df.loc[index].flag:

As sometimes the flag is np.nan, which evaluates to true-ish*, suddenly the branch started executing when we really did not want it to execute (hence the explicit == True in our code). * I so wish it did not, but anyway, it does

Please do not perform this change, it can and does lead to actual problems in the codebase!

Many thanks

adam-ah avatar Mar 14 '21 02:03 adam-ah

I came here because this introduced a bug in our codebase too. It is very easy to reproduce this issue. Just check the output of this pre and post autopep8:

def equals_true(arg) -> bool:
    return arg == True

print(equals_true(None))

StefanoPietrosanti avatar Oct 04 '23 09:10 StefanoPietrosanti

OMG I can't believe this is still an open bug more than 3 years after reporting it

adam-ah avatar Oct 04 '23 09:10 adam-ah