Desired change

• Rule(s): SIM102
• Adjustment: Ignore if __name__ == "__main__"

Explanation

the check if __name__ == "__main__" is special. People recognize it as a special part of a script. Although one can combine things, I think it's better not to do it.

Example

The following should not trigger SIM102

if __name__ == "__main__":
    if foo(): ...

Desired change

• Rule(s): SIM113
• Adjustment: Don't trigger SIM113 in nested loops if the inner loop is counting

Example

# Code which should NOT trigger SIM113
nb_points = 0
for line in lines:
    for point in line:
        nb_points += 1

Desired change

• Rule(s): SIM102
• Adjustment: If a comment is placed between if conditions, don't flag this rule

Explanation

Sometimes it is clearer to split up logic into separate if conditions with comments explaining what is going on under the if conditions

Example

This is an example where the mentioned rule(s) would currently be suboptimal:

for p in reversed(path.parents):
    ...
    if p == path.parent:
        # This is the last iteration
        if p in some_list and some_bool:
            do_something()

With the current rule, I guess you can do this:

for p in reversed(path.parents):
    ...
    if (
        p == path.parent
        # This is the last iteration
        and
        p in some_list and some_bool
    ):
        do_something()

but it is less readable in my opinion.

Rule

Do not use an if-else block when you can directly return the condition.

Example

# Bad
if condition:
    return True
else
    return False

# Good
return bool(condition)

Explanation

The first one is shorter and extremely wide-spread

Example

# Bad
key in some_dict.keys()

# Good
key in some_dict

Explanation

Less indentation is simpler

Example

# Bad
            elif get_app().window.actionFilesShowAudio.isChecked():
                if media_type != "audio":
                    return False

# Good
            elif get_app().window.actionFilesShowAudio.isChecked() and media_type != "audio":
                return False

Explanation

Positional boolean arguments are typically hard to understand, because they can represent so many things. Having a keyword-argument gives the reader the keyword name as context.

Example

import zipfile

# Bad
fp = zipfile.ZipFile("filename", "w", zipfile.ZIP_DEFLATED, False)

# Good
fp = zipfile.ZipFile("filename", "w", zipfile.ZIP_DEFLATED, allowZip64=False)

Desired change

• Rule(s): SIM110
• Adjustment: Fix issue

Example

This is an example where the mentioned rule(s) would currently be suboptimal:

#  Code
for el in [1,2,3]:
    if is_true(el):
        return True
raise Exception

# Current message
SIM110 Use 'return any(is_true(el) for el in [1, 2, 3])'

# Desired message: Either none or the following:
SIM110 Use 'if any(is_true(el) for el in [1, 2, 3]): return True; raise Exception'

Explanation

• If you use a dictionary, you can easily see all expected parameters with `dict.keys()
• The formatting typically is slightly cleaner

Example

# Bad
if a == "foo":
    return "bar"
elif a == "bar":
    return "baz"
elif a == "boo":
    return "ooh"
else:
    return 42

# Good
mapping = {"foo": "bar", "bar": "baz", "boo": "ooh"}
return mapping.get(a, 42)

Desired change

• Rule(s): SIM111
• Adjustment: Make sure the recommended change is correct.

Explanation

The current recommended change is wrong.

Example

This is an example where the mentioned rule(s) would currently be suboptimal:

# Original
for interval in self.intervals:
    if not interval.is_empty():
        return False
return True

# Current replacement
return all(not interval.is_empty() for interval in self.intervals)

# Correct
return all(interval.is_empty() for interval in self.intervals)

Explanation

It is pretty common to have CONSTANT_STYLE environment variables.

Example

# Bad
os.environ["foo"]
os.environ.get("bar")

# Good
os.environ["FOO"]
os.environ.get("BAR")

Desired change

• Rule(s): SIM113
• Adjustment: The rule should see whether the right side of the increment is a number.

Explanation

Just looking at whether a variable is being incremented in a loop isn't enough to consider enumerate().

Example

This triggers SIM113 Use enumerate instead of 'line':

for line in read_list(redis_conn, storage_key):
    line += b'\n'
    # ....

Desired change

• Rule(s): SIM113
• Adjustment: Only suggest enumerate when +=1 is executed unconditionally.

Explanation

If you pre-filter the things you iterate on before adding 1 to the total, enumerate will have a different number than current total.

Example

This is an example where the mentioned rule(s) would currently be suboptimal:

odd_x = 0
for x in [1, 2, 3]:
    if x % 2:
        continue
    odd_x += 1

Explanation

Using a context handler is shorter and avoids the error of forgetting to close a file handle.

Example

# Bad
f = open(...) 
... # (do something with f)
f.close() 

# Good
with open(..) as f:
   ... # (do something with f)

Explanation

There are many opportunities for the walrus operator to simplify code, but my go-to party trick is below.

Example

# Bad
with open('file.txt', 'r') as fd:
  chunk = fd.read(1024)
  while chunk:
    do_stuff(chunk)
    chunk = fd.read(1024)

# Good
with open('file.txt', 'r') as fd:
  while (chunk := fd.read(1024)):
    do_stuff(chunk)

In general:

# Bad
x = ...
# no other use of x
if x:  # or while, if it assigns x in a similar way
    # do stuff with x
# no further use of x

# Good
if (x := ...):  # or while, if it assigns x in a similar way
    # do stuff with x
# no further use of x

Explanation

See https://sourcery.ai/blog/explaining-refactorings-4/

Example

# Bad
if a:
    b
elif c:
    b

# Good
if a or c:
    b

Desired change

• Rule(s): SIM118
• Adjustment: Don't trigger if keys are removed within the loop

Explanation

The change would actually introduce a bug

Example

This is an example where the mentioned rule(s) would currently be suboptimal:

# Before
for key in list(dict.keys()):
    if some_property(key):
        del dict[key]

# After (wrong?)
for key in dict:
    if some_property(key):
        del dict[key]

Explanation

It's way more common to have the variable first.

Example

# Bad
"Yoda" == i_am

# Good
i_am == "Yoda"

Desired change

• Rule(s): SIM119
• Adjustment: SIM119 should be more precise.

Explanation

Examples below are suggested as dataclass. I think they shouldnt

Example

class RegexAssert:
    def __init__(self, pattern, flags=0):
        self._regex = re.compile(pattern, flags)

    def __eq__(self, actual):
        return bool(self._regex.match(actual))

    def __repr__(self):
        return self._regex.pattern

class OfType:
    """
    >>> 3 == OfType(int, str, bool)
    True
    >>> 'txt' == OfType(int)
    False
    """

    def __init__(self, *types):
        self.type = types

    def __eq__(self, other):
        return isinstance(other, self.type)

Explanation

Use 'enumerate' to get a running number over an iterable.

Example

# Bad
idx = 0
for el in iterable:
    ...
    idx += 1

# Good
for idx, el in enumerate(iterable):
    ...

Explanation

Analyze f-strings. If f-string has no text and only a passed value, then use that value directly.

Example 1

# Bad

foo = "bar"
my_text = f"{foo}"

# Good

foo = "bar"
my_text = foo

Example 2

# Bad

def my_func(foo: str):
    my_text = f"{foo}"

# Good
def my_func(foo: str):
    my_text = foo

Example 3

# Bad

my_number = 123
my_text = f"{foo}"

# Good

my_number = 123
my_text = str(foo)

Desired change

• Rule(s): SIM116
• Adjustment: Don't suggest a dictionary when the value is a side-effecting function call.

Explanation

Constructing a dictionary of all values will invoke all side effects from all branches, which is not desired.

On top of that, the suggestion wrongly quotes the values.

Example

class Cat:
    weight = 10
    sound = 'purr'


def launch(launchable):
    print('{} have been launched!'.format(launchable))
    return len(launchable)


def pet(animal):
    print(animal.sound + '!')
    return animal.weight


def example(action, **kwargs):
    if action == 'launch':
        return launch('Rockets')
    elif action == 'pet':
        return pet(kwargs['animal'])
    elif action == 'idle':
        return None


# Bad suggestion:
# SIM116 Use a dictionary lookup instead of 3+ if/elif-statements: return {'launch': "launch('Rockets')", 'pet': "pet(kwargs['animal'])", 'idle': 'None'}.get(action)


def bad_suggestion(action, **kwargs):
    return {
        'launch': launch('Rockets'),
        'pet': pet(kwargs['animal']),
        'idle': None,
    }.get(action)

>>> bad_suggestion('pet', animal=Cat())
Rockets have been launched!
purr!

Desired change

Rules for reducing the usage of "not" should not be applied if an exception is raised if the condition applies.

Explanation

The pattern usually is:

if not condition:
    raise Exception("condition is not fulfilled but needs to be")

Example

if not (minimum < maximum):
    raise ValueError(f"minimum={minimum} needs to be smaller than maximum={maximum}")

Explanation

The return value might be unexpected in a try-except-finally clause. It is better to avoid a return in those cases

Example

# Bad: This example returns 3!
def foo():
    try:
        1 / 0
        return "1"
    except:
        return "2"
    finally:
        return "3"

# Good
def foo():
    return_value = None
    try:
        1 / 0
        return_value = "1"
    except:
        return_value = "2"
    finally:
        return_value = "3"  # you might also want to check if "except" happened
    return return_value

Rule

Use contextlib.suppress instead of try-except-pass

Example

# Bad
try:
    foo()
except ValueError:
    pass

# Good
from contextlib import suppress

with suppress(ValueError):
    foo()

Rule

Use yield from if possible

Example

# Bad
for item in iterable:
    yield item

# Good
yield from iterable

Desired change

• Rule(s): SIM115
• Adjustment: Accept usage of contextlib.ExitStack

Explanation

In some applications, we may have a dynamic number of contextmanagers which we may choose to use contextlib.ExitStack to cleanup. This does not satisfy SIM115

Example

This is an example where the mentioned rule(s) would currently be suboptimal:

import contextlib

files_to_read = ['/tmp/foo', '/tmp/bar']

with contextlib.ExitStack() as stack:
    files = [stack.enter_context(open(path)) for path in files_to_read]
    # do stuff to files

The above example unfortunately doesn't satisfy SIM115, even though it takes care of exiting.

Explanation

Removing a negation makes things simpler to understand

Example

# Bad
b if not a else a

# Good
a if a else b

Desired change

• Rule(s): SIM111
• Adjustment: Detect if another statement comes after the loop

Explanation

The below code is flagged with SIM111, but if I would would use the suggested code, the call_method call would be lost.

Example

for a in my_list:
  if a == 2:
    return False
call_method()
return True

Rule

Use the condition directly (or bool(condition)) to get a boolean. Do not use the ternary operator.

Example

# Bad
True if condition else False

# Good
condition

Explanation

Exceptions and error cases should be handled early to prevent deeply-nested code.

Example

# Bad
if cond:
    a
    b
    c
else:
    raise Exception

# Good
if not cond:
    raise Exception

a
b
c

Desired change

• Rule(s): SIM104
• Adjustment: do not propose yield from on async generators.

Explanation

Yield-from is strictly forbidden under async generators, it looks like flake8-simplify does not check if generator is async before proposing that change.

Example

async def items():
    for c in 'abc':
        yield c

Explanation

Most of the discussion of the motivation about this rule can be found in this very detailed StackOverflow post

Example

# Bad
key in dict.keys()

# Good
key in dict

Basically, the later way of doing this pythonic and slightly more performant and arguably more Readable. The previous method only exists for legacy code (<= python 2.2). So there is no reason to use the former in modern codebases.

Explanation

A little known feature of Python is that a with statement can contain multiple context managers. This allows the code only one with statement (and therefore only 1 level of indentation). This rule has a similar rational to SIM102.

This rule should be applied if and only if:

• context A only contains the code of Context B and no other code. That is all the code in the nested statement will run with context A and B.

Caveat when implementing: If the context names are really long, the with statement may be broken over a line break. A new feature in the Python 3.10 alpha will be to allow parentheses to be used to break the with statement over multiple lines.

Example

Consider the following context managers:

#bad
with A() as a:
    with B() as b:
        print('hello')

can be transformed into the following:

# Good
with A() as a, B() as b:
    print('hello')

Desired change

SIM113 False Positive in nested loops

In this case, we could using enumerate is rather tricky. How do you suggest we best do it in nested loops like so?

• Rule(s): SIM113
• Adjustment: In which way should it/they be adjusted?
  The result should not be triggered in nested loops

Explanation

Why is the adjustment necessary / better?
Because the enumerate logic actually becomes MORE complicated in this nested loop context. You need to have some complex multiplication to get the proper number and that may not even be possible if bars is a generator that has variable length. In which case, counting manually here is actually for the best.

Example

This is an example where the mentioned rule(s) would currently be suboptimal:

count = 0
for foo in foos:
    for bar in bars:
        count +=1

In this second one, I could se the argument for epochs (assuming there is not complex increment logic, but it complains about number of iterations)

epoch, i = np.load(resume_file)
while epochs < TOTAL_EPOCHS:
    for b in batches:
        i+=1

Explanation

Sometimes conditional expressions can be replaced with boolean ones, like this:

• a if a else b -> a or b
• b if a else a -> a and b

This makes the code shorter and removes redundant evaluations of expressions. I think a check to suggest such replacements would fit well in this plugin.

Example

# Bad
x = o.x if o.x else default_x

# Good
x = o.x or default_x

Desired change

• Rule(s): SIM104
• Adjustment: do not raise a warning inside async functions

Explanation

Python does not support 'yield from' inside async functions

https://www.python.org/dev/peps/pep-0525/#asynchronous-yield-from

If 'flake8-simplify' already can check if a function is "async" or not, then the issue has to be easy to fix.

Example

This is an example where the mentioned rule(s) would currently be suboptimal:

Copied from the https://www.python.org/dev/peps/pep-0525/#asynchronous-yield-from:

async def g2():
    async for v in g1():
        yield v

Desired change

• Rule(s): SIM113
• Adjustment: False-positive: Do not trigger if continue changes the number

Example

even_numbers = 0
for el in range(100):
    if el % 2 == 1:
        continue
    even_numbers += 1

Explanation

The get(key, default_fallback=None) method of a dictionary is often overlooked. It provides a clear interface that every Python developer should know.

Example

# Bad
name = "some_default"
if "some_key" in some_dict:
    name = some_dict["some_key"]

# Good
name = some_dict.get("some_key", "some_default")

Desired change

• Rule(s): SIM106
• Adjustment: Make an exception for the NotImplementedError

Explanation

Throwing an exception for potentially forgotten implementation is better than potentially returning None or making the last one catch all.

Example

    if merge_method in ["take_right_shallow", "take_right_deep"]:
        ...
    elif merge_method == "take_left_shallow":
        ...
    elif merge_method == "take_left_deep":
        ...
    elif merge_method == "sum":
        ...
    else:
        raise NotImplementedError

Desired change

• Rule(s): SIM114
• Adjustment: Remove false positive findnings

Explanation

The current one is wrong 😄

Example

def complicated_calc(*arg, **kwargs):
    return 42

def foo(p):
    if p == 2:
        return complicated_calc(microsecond=0)
    elif p == 3:
        return complicated_calc(microsecond=0, second=0)
    return None

dummy.py:5:8: SIM114 Use logical or ((p == 2) or (p == 3)) and a single body

Explanation

It's shorter.

Example 1: any

# Bad
for x in iterable:
    if check(x):
        return True
return False

# Good
return any(check(x) for x in iterable)

Example 2: all

# Bad
for x in iterable:
    if check(x):
        return False
return True

# Good
return all(check(x) for x in iterable)

Explanation

Sometimes people tend to write Union[Type, None] instead of Optional[Type].

Example

# Bad
def foo(a: Union[int, None]) -> Union[int, None]:
  return a

# Good
def foo(a: Optional[int]) -> Optional[int]:
  return a

Explanation

Dataclasses were introduced with PEP 557. Data Classes can be thought of as "mutable namedtuples with defaults".

Dataclasses should be used when the primary focus is to store data.

Dataclasses help, because they have a good __str__ and __eq__ implementation and you don't have to write __init__.

Example

# Bad
class Person:
    def __init__(self, first_name, last_name, birthdate):
        self.first_name = first_name
        self.last_name = last_name
        self.birthdate = birthdate


# Good
from dataclasses import dataclass
from datetime import date


@dataclass
class Person:
    first_name: str
    last_name: str
    birthdate: date

As an example why dataclasses are nice:

p11 = Person1("Bob", "Builder", date(2000, 1, 3))
p12 = Person1("Bob", "Builder", date(2000, 1, 3))
p21 = Person2("Bob", "Builder", date(2000, 1, 3))
p22 = Person2("Bob", "Builder", date(2000, 1, 3))

print(p11)
print(p11 == p12)
print("-" * 80)
print(p21)
print(p21 == p22)

gives:

<__main__.Person1 object at 0x7f3f8a32aac0>
False
--------------------------------------------------------------------------------
Person2(first_name='Bob', last_name='Builder', birthdate=datetime.date(2000, 1, 3))
True

Several issues are blocked, because I don't know yet how to get context.

For example, SIM 201 prevents not a==b and suggests a != b. In general, this seems to be a good rule.

As an exception, I want to leave it like this when it's in the context of throwing exceptions:

if not (a == b):
    return ValueError("Expected a==b")

But for this I need to know that the boolean operation not (a ==b) is in the context of an if-statement.

Explanation

Sometimes people make parenthesis around expressions that definitely don't need them. It just adds optical clutter without adding value. For this reason, the parenthesis should be removed.

Example

# Bad
msg = ("foo")

# Good
msg = "foo"

Explanation

Use the "in" operator and a list / tuple instead of multiple comparisons.

Example

# Bad
if a == b or a == c:
    d

# Good
if a in [b, c]:
    d

Explanation

Use the ternary operator if it's reasonable.

Example

# Bad
if a:
    b = c
else:
    b = d

# Good
b = c if a else d

Desired change

• Rule(s): NA
• Adjustment: This is a bug report, not an adjustment.

Explanation

When I execute flake8 using pytest like this pytest --flake8 --mypy I get the following segfault:

Current thread 0x00007ffff7ff4740 (most recent call first):
  File "venv/lib/python3.6/site-packages/flake8_simplify.py", 
line 941 in is_stmt_equal
  File "venv/lib/python3.6/site-packages/flake8_simplify.py", 
line 947 in is_stmt_equal
...
  File "venv/lib/python3.6/site-packages/flake8_simplify.py", 
line 947 in is_stmt_equal
  File "venv/lib/python3.6/site-packages/flake8_simplify.py", 
line 947 in is_stmt_equal
  ...
Segmentation fault

I know this error may not give enough information but I have no idea how to debug this. Also, I noticed this happens only in versions 0.12.0 to 0.14.1, but not in 0.11.0 and only with the exact parameters mentioned above. If for instance I run pytest --flake8 or just flake8 . --exclude venv the problem will not occur.

Desired change

• Rule(s): SIM111
• Adjustment: Fix warning message for compound conditionals

Explanation

The warning message is incorrect for compound expressions. It can be fixed by enclosing the conditional in parentheses.

Example

This is an example where the mentioned rule(s) would currently be suboptimal wrong:

for x in iterable:
    if A(x) and B(x):
        return False
return True

Result, with correction:

- SIM111: Use 'return all(not A(x) and B(x) for x in iterable)'
+ SIM111: Use 'return all(not (A(x) and B(x)) for x in iterable)'
                              ^             ^

or (but you probably don't want to be parsing the expression and doing boolean algebra?):

- SIM111: Use 'return all(not A(x) and B(x) for x in iterable)'
+ SIM111: Use 'return all(not A(x) or not B(x) for x in iterable)'