Python maintainability best practice

Code Review Doctor suggests a fix if your code does not follows Python maintainability best practices.

This frees your team to be more agile - they can focus on adding value rather than fighting with old tech debt.

Money
code-review-doctorbotsuggested changes just now
helpers.py
1
+
class FooBarClass(object):
Suggested changes
-
class FooBarClass(object):
+
class FooBarClass:
Commit suggestion

A class is inherited from object by default, so explicitly inheriting from object is redundant. Removing it keeps the code simpler.

Read more
2
+
    pass
validation.py
1
+
if isinstance(item, int) or isinstance(item, float):
Suggested changes
-
if isinstance(item, int) or isinstance(item, float):
+
if isinstance(item, (int, float)):
Commit suggestion

isinstance can take multiple types, so there is no need to call them multiple times for each type.

Read more
2
+
    pass
validation.py
1
+
if isinstance(item, int) or isinstance(item, float):
Suggested changes
-
if isinstance(item, int) or isinstance(item, float):
+
if isinstance(item, (int, float)):
Commit suggestion

isinstance can take multiple types, so there is no need to call them multiple times for each type.

Read more
2
+
    pass
models.py
1
+
class FooBarClass(NamedTuple):
Suggested changes
-
class FooBarClass(NamedTuple):
+
@dataclass(frozen=True)
+
class FooBarClass:
    pass
Commit suggestion

Consider using the dataclass here instead for simplicity, performance gains, and consistency

Read more
2
+
    pass
helpers.py
1
+
class FooBarClass(Bar):
2
+
    def foo(self, bar):
3
+
        super(Foo, self).foo(bar)
Suggested changes
-
        super(Foo, self).foo(bar)
+
        super().foo(bar)
Commit suggestion

It's unnecessary to use arguments when calling super for the parent class.

Read more
helpers.py
1
+
value = f'some-value'
Suggested changes
-
value = f'some-value'
+
value = 'some-value'
Commit suggestion

f-string is unnecessary here as there are no placeholders in the string.

Read more
helpers.py
1
+
foo = lambda x: bar(x)
Suggested changes
-
foo = lambda x: bar(x)
+
foo = bar
Commit suggestion

Avoid unnecessarily wrapping a function in a lambda

Read more
helpers.py
1
+
any([item for item in items])
Suggested changes
-
any([item for item in items])
+
any(item for item in items)
Commit suggestion

all and any can take a generator, so constructing a list first may be unnecessary.

Read more
helpers.py
1
+
def foo_bar() -> "FooBarClass":
Suggested changes
-
def foo_bar() -> "FooBarClass":
+
def foo_bar() -> FooBarClass:
Expand 4 lines ...
Commit suggestion

Use type identifiers instead of using string type hints.

Read more
2
+
    return FooBarClass()
3
+
	
4
+
class FooBarClass:
5
+
    pass
helpers.py
1
+
def foo_bar() -> Union[int, None]:
Suggested changes
-
def foo_bar() -> Union[int, None]:
+
def foo_bar() -> Optional[int]:
Commit suggestion

This could be simplified by using typing.Optional

Read more
2
+
    # sometimes return int other times None
helpers.py
1
+
dict((item, get_value(item)) for item in items)
Suggested changes
-
dict((item, get_value(item)) for item in items)
+
{item: get_value(item) item item in items}
Commit suggestion

Using list and dict comprehension is simpler and computationally quicker than calling list() and dict().

Read more
helpers.py
1
+
values = list([1, 2, 3])
Suggested changes
-
values = list([1, 2, 3])
+
values = [1, 2, 3]
Commit suggestion

Using list/set/dict literal syntax is simpler and computationally quicker than calling list(), set(), or dict().

Read more
helpers.py
1
+
def get_name(user):
2
+
    return "%s %s" % (user.first_name, user.last_name)
Suggested changes
-
    return "%s %s" % (user.first_name, user.last_name)
+
    return f'{user.first_name} {user.last_name}'
Commit suggestion

f-string is easier to read, write, and less computationally expensive than legacy string formatting.

Read more
helpers.py
1
+
from typing import Optional
2
+
	
3
+
def foo_bar(value: Optional[str] = None):
4
+
    if value:
Suggested changes
-
    if value:
+
    if value is not None:
Commit suggestion

Checking Optional variables against None would be more explicit.

Read more
5
+
        ...
catalogue.py
1
+
with open('some/path.json') as f:
2
+
    content = json.loads(f.read())
Suggested changes
-
    content = json.loads(f.read())
+
    content = json.load(f)
Commit suggestion

json.load(f) simplifies reading JSON from disk.

Read more
catalogue.py
1
+
with open('some/path.json', 'w') as f:
2
+
    f.write(json.dumps({'foo': 'bar'}))
Suggested changes
-
    f.write(json.dumps({'foo': 'bar'}))
+
    json.dump({'foo': 'bar'}, f)
Commit suggestion

json.dump(f) simplifies writing JSON to disk.

Read more
Update catalogue.py

Are you ready to improve your team agility through lower tech debt? Add Code Review Doctor to GitHub.