Ревью Python: замечания, которые я пишу чаще всего

За несколько лет ревью на Python набирается список замечаний, которые повторяются из PR в PR независимо от команды и проекта. Ниже — тридцать формулировок, которые я пишу в комментариях чаще всего, с объяснением, почему это замечание, а не придирка, и как выглядит правильный вариант. Ориентир — Python 3.11+, но большинство пунктов актуальны с 3.8. Это разбор для тех, кто сам ревьюит или готовит код к ревью: конкретные формулировки замечаний вместо абстрактного «поработайте над качеством».

Формулировки не про стиль вроде длины строк — это дело ruff или black. Речь про то, что ломается в проде или мешает читать, а автоформатер молчит.

Корректность: то, что взрывается в рантайме

1. Изменяемый аргумент по умолчанию

Замечание: «def f(items=[]) — список создаётся один раз при определении функции и разделяется между всеми вызовами. Используйте None и создавайте внутри».

# было
def add(item, target=[]):
    target.append(item)
    return target

# стало
def add(item, target=None):
    if target is None:
        target = []
    target.append(item)
    return target

То же касается {} и любого мутабельного объекта в дефолте. Это одна из самых частых причин «загадочного» накопления данных между вызовами.

2. Голый except или except Exception без нужды

Замечание: «except: поймает и KeyboardInterrupt, и SystemExit — процесс перестанет останавливаться по Ctrl+C. Ловите конкретный класс исключения».

# плохо
try:
    value = data[key]
except:
    value = None

# хорошо
try:
    value = data[key]
except KeyError:
    value = None

3. Проглоченное исключение без логирования

Замечание: «В блоке except стоит pass — если здесь что-то упадёт, вы никогда об этом не узнаете. Либо логируйте, либо прокидывайте дальше».

4. Ресурс не закрывается

Замечание: «Файл (соединение, сокет) открыт напрямую — при исключении он не закроется. Используйте with».

# было
f = open(path)
data = f.read()
f.close()  # не выполнится при исключении в read()

# стало
with open(path, encoding="utf-8") as f:
    data = f.read()

5. open без указания кодировки

Замечание: «open(path) без encoding берёт кодировку из локали ОС. На Windows это часто cp1251, и файл прочитается с ошибками. Указывайте encoding="utf-8" явно».

6. Сравнение с None через ==

Замечание: «Используйте is None, а не == None. Оператор == может быть переопределён у объекта и вернуть неожиданный результат».

7. Проверка на пустоту через len

Замечание: «if len(items) == 0 заменяется на if not items — короче и работает для любых коллекций». Обратная сторона: если объект может быть None, not items сработает и для None, и для пустого списка — это надо учитывать.

8. Мутация коллекции во время итерации по ней

Замечание: «Вы удаляете элементы из списка внутри for по этому же списку — часть элементов пропустится. Итерируйтесь по копии или собирайте новый список».

Читаемость и API функций

9. Функция делает три дела сразу

Замечание: «Функция читает файл, парсит и пишет в базу. По имени process непонятно, что она делает. Разбейте на три и назовите по действию».

10. Булев флаг-параметр

Замечание: «В вызове render(user, True, False) непонятно, что означают эти True и False. Сделайте их keyword-only через * или замените на enum».

# звёздочка запрещает передавать флаги позиционно
def render(user, *, compact=False, cache=True):
    ...

# теперь вызов читается сам
render(user, compact=True, cache=False)

11. Слишком много позиционных аргументов

Замечание: «Пять позиционных аргументов подряд — на месте вызова их легко перепутать. Сгруппируйте связанные в dataclass».

12. Отсутствуют аннотации типов у публичной функции

Замечание: «Функция экспортируется из модуля, но без аннотаций. IDE и mypy не подскажут ошибку у вызывающего. Добавьте типы хотя бы для сигнатуры».

13. Аннотация Any вместо конкретного типа

Замечание: «data: Any отключает проверку типов на этой переменной целиком. Если это словарь известной формы — опишите через TypedDict или dataclass».

14. Магическое число без имени

Замечание: «В коде timeout=86400 — что это за число, придётся считать в уме. Вынесите в константу с говорящим именем: SECONDS_IN_DAY = 86400».

15. Закомментированный код

Замечание: «Закомментированный блок — удалите. История в git, восстановить можно всегда, а мёртвый код только сбивает при чтении».

16. Комментарий пересказывает строку

Замечание: «Комментарий # увеличиваем счётчик над counter += 1 ничего не добавляет. Комментарий должен объяснять почему, а не что».

Идиомы, которые превращают три строки в одну

17. Ручной обход индексов вместо enumerate

Замечание: «for i in range(len(items)) с последующим items[i] заменяется на for i, item in enumerate(items)».

18. Ручное построение словаря вместо comprehension

# многословно
result = {}
for user in users:
    result[user.id] = user.name

# идиоматично
result = {user.id: user.name for user in users}

19. Конкатенация строк в цикле через +=

Замечание: «Каждое result += chunk в цикле создаёт новую строку — на больших объёмах это квадратичная сложность. Собирайте в список и делайте "".join(parts) в конце».

20. Обращение к словарю с проверкой if key in d

Замечание: «d[key] if key in d else default — это d.get(key, default). Для накопления — d.setdefault или collections.defaultdict».

21. Ручная распаковка вместо zip или распаковки

Замечание: «first = pair[0]; second = pair[1] — распакуйте сразу: first, second = pair».

f-строки, форматирование и логи

22. Старое форматирование % или .format

Замечание: «С Python 3.6 есть f-строки — они читаются лучше и быстрее. Замените "%s" % name на f"{name}"». Исключение — пункт 23.

23. f-строка в вызове логгера

Замечание: «logger.info(f"user {user_id} done") форматирует строку всегда, даже если уровень INFO выключен. Передавайте аргументы отдельно — логгер подставит их только если запись реально пишется».

# дороже: f-строка вычисляется до вызова
logger.debug(f"payload: {expensive_dump(obj)}")

# дешевле: аргумент вычисляется, но подстановка ленивая
logger.debug("payload: %s", obj)

Здесь есть нюанс: expensive_dump(obj) во втором варианте всё равно вычислится, если передать его как аргумент. Ленивость касается только форматирования, а не вычисления самих аргументов.

24. Секрет или токен в коде

Замечание: «В коде хардкод API_KEY = "...". Даже если ключ тестовый — уберите в переменные окружения или секрет-хранилище. В git такое остаётся навсегда».

Тесты и надёжность

25. Тест без assert

Замечание: «Тест вызывает функцию, но ничего не проверяет — он зелёный, только если код не бросает исключение. Добавьте проверку результата».

26. Тест зависит от порядка запуска

Замечание: «Тест использует состояние, оставленное предыдущим. При запуске в одиночку он падает. Изолируйте через фикстуру».

27. Сравнение float через ==

Замечание: «assert result == 0.1 + 0.2 нестабилен из-за представления чисел с плавающей точкой. Используйте math.isclose или pytest.approx».

Асинхронность и производительность

28. Блокирующий вызов внутри async-функции

Замечание: «В async def вы делаете синхронный requests.get или time.sleep — они блокируют весь event loop, и остальные корутины встают. Используйте async-клиент или вынесите в run_in_executor».

29. await в цикле там, где нужен gather

Замечание: «Вы делаете await fetch(url) по очереди в цикле — запросы идут последовательно. Если они независимы, соберите через asyncio.gather и выполните параллельно».

30. N+1 запросов к базе

Замечание: «В цикле по объектам вы дёргаете базу на каждой итерации. Для тысячи объектов это тысяча запросов. Загрузите одним запросом с IN или через select_related / prefetch_related, если это ORM».

Хорошее замечание объясняет, что сломается и почему, и даёт правильный вариант. «Так не пишут» без причины автор проигнорирует и будет прав.

Как отделить блокирующее от косметики

Не все тридцать пунктов одинаково срочны. Разделите замечания на группы, чтобы автор понимал, что чинить обязательно, а что — по желанию.

Помечайте необязательные замечания префиксом вроде nit: — автор сразу видит, что это предложение, а не блокер. Это снижает трение: спор о запятой не должен весить столько же, сколько утечка соединения.

Что автоматизировать, чтобы не писать это руками

Большая часть косметики выносится в инструменты, и тогда на ревью остаётся только то, что требует головы:

• ruff — ловит пункты 1, 2, 6, 17, 22 и десятки других правил flake8/pyupgrade в одном быстром линтере. Ставится в pre-commit.
• mypy или pyright — закрывает пункты 12 и 13, находит несоответствия типов до рантайма.
• pre-commit хук — прогоняет ruff и форматер до коммита, чтобы стилевые замечания вообще не доходили до ревьюера.

Точный набор правил ruff и синтаксис конфига смотрите в актуальной документации — набор кодов правил меняется между версиями. Смысл в том, что ревью тем полезнее, чем меньше на нём обсуждают то, что проверяет машина.

Что делать дальше

Настройте pre-commit с ruff и mypy на проекте — большинство пунктов из групп «идиомы» и «читаемость API» отвалятся автоматически, и вы сможете сосредоточиться на первых восьми и на асинхронности, где линтер бессилен. Заведите в команде короткий чек-лист блокирующих замечаний (корректность, утечки, тесты) и договоритесь о префиксе nit: для необязательного. Ревью пойдёт быстрее, а споры о том, что важнее — станут реже.